Skip to content

Lazy load legacy imports in skimage top module #6892

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 55 additions & 112 deletions skimage/__init__.py
Original file line number Diff line number Diff line change
@@ -1,68 +1,11 @@
"""Image Processing for Python

``scikit-image`` (a.k.a. ``skimage``) is a collection of algorithms for image
scikit-image (a.k.a. ``skimage``) is a collection of algorithms for image
processing and computer vision.

The main package of ``skimage`` only provides a few utilities for converting
between image data types; for most features, you need to import one of the
following subpackages:

Subpackages
-----------
color
Color space conversion.
data
Test images and example data.
draw
Drawing primitives (lines, text, etc.) that operate on NumPy arrays.
exposure
Image intensity adjustment, e.g., histogram equalization, etc.
feature
Feature detection and extraction, e.g., texture analysis corners, etc.
filters
Sharpening, edge finding, rank filters, thresholding, etc.
graph
Graph-theoretic operations, e.g., shortest paths.
io
Reading, saving, and displaying images and video.
measure
Measurement of image properties, e.g., region properties and contours.
metrics
Metrics corresponding to images, e.g. distance metrics, similarity, etc.
morphology
Morphological operations, e.g., opening or skeletonization.
restoration
Restoration algorithms, e.g., deconvolution algorithms, denoising, etc.
segmentation
Partitioning an image into multiple regions.
transform
Geometric and other transforms, e.g., rotation or the Radon transform.
util
Generic utilities.

Utility Functions
-----------------
img_as_float
Convert an image to floating point format, with values in [0, 1].
Is similar to `img_as_float64`, but will not convert lower-precision
floating point arrays to `float64`.
img_as_float32
Convert an image to single-precision (32-bit) floating point format,
with values in [0, 1].
img_as_float64
Convert an image to double-precision (64-bit) floating point format,
with values in [0, 1].
img_as_uint
Convert an image to unsigned integer format, with values in [0, 65535].
img_as_int
Convert an image to signed integer format, with values in [-32768, 32767].
img_as_ubyte
Convert an image to unsigned byte format, with values in [0, 255].
img_as_bool
Convert an image to boolean format, with values either True or False.
dtype_limits
Return intensity limits, i.e. (min, max) tuple, of the image's dtype.

between image data types; for most features, you need to import one of its
subpackages.
"""

__version__ = '0.25.0rc2.dev0'
Expand All @@ -73,7 +16,33 @@


def __dir__():
return __lazy_dir__() + ['__version__']
"""Add lazy-loaded attributes to keep consistent with `__getattr__`."""
patched_dir = {*globals().keys(), *__lazy_dir__()}
return sorted(patched_dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is fixed by this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lazy loading expands the module attributes that are available via __getattr__. These new attributes are missing from the default __dir__() / dir(skimage). If we use __dir__ returned from attach_stub, then a whole bunch of pre-existing attributes like __file__, __doc__, etc. are missing. Both are inconsistent and it seemed like a good opportunity to preempt weird or confusing behavior by code inspection tools, etc. that don't expect messing with __dir__.

Spelling this out, I realize that this is also a problem with our subpackages that just use __dir__ from attach_stub as is. Not sure if it's worth it to go through all these. Neither am I sure how to conveniently address this from lazy_loader's site. globals() returns the scope from where the function is defined, so lazy_loader would need a different way to access the correct global scope...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we want to expose things like __file__ etc.? __dir__ looks exactly like what I would expect atm:

In [1]: dir(ski)
Out[1]:
['__version__',
 'color',
 'data',
 'draw',
 'exposure',
 'feature',
 'filters',
 'future',
 'graph',
 'io',
 'measure',
 'metrics',
 'morphology',
 'registration',
 'restoration',
 'segmentation',
 'transform',
 'util']

You can use various hacks to get hold of other attributes, but these are the ones we want our users to see.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you really need to expose more (and I don't think we should), then you could:

def __dir__():
    return [some stuff] + __lazy_dir__()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we want to expose things like file etc.?

For the reasons stated above. Hiding builtin attributes feels very magical and unexpected to me, and like something that might lead confusing bugs. I'd like tab-completion in ipython to show me everything that's available and not a curated list.

Everyone is aware that _ means private and is used to __builtin_stuff__. Why would we need to hide these?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like I do recognize all your arguments, I just don't find them as convincing as you do.

This is a common problem I run into 🙃

Since you'd like to see a change in behavior, I think you should make a convincing case for (a) why this is useful (and not confusing) to the user and (b) that it does not introduce unnecessary complexity (we could improve the code to make that the case, so I'm not too worried about this part).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe more importantly, we are way overdue on the release, so a friendly request that we merge this PR without the change to __dir__ and then do additional follow-up once we come to an agreement.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added it as a talking point to today's meeting notes.

Anyway, I'm happy to iterate on this particular issue in another PR. And I'm not seeing this issue as a blocker for the release in the first place.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted to status quo in 0a275cd with regards to hiding stuff in __dir__.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python's docs seem to be compatible with your stance @stefanv:

The resulting list is not necessarily complete and may be inaccurate when the object has a custom __getattr__().

[...] it attempts to produce the most relevant, rather than complete, information: [...]

Because dir() is supplied primarily as a convenience for use at an interactive prompt, it tries to supply an interesting set of names more than it tries to supply a rigorously or consistently defined set of names, and its detailed behavior may change across releases.



# `attach_stub` currently ignores __all__ inside the stub file and simply
# returns every lazy-imported object, so we need to define `__all__` again.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See scientific-python/lazy-loader#133 for a proposal to address this duplication.

__all__ = [
'color',
'data',
'draw',
'exposure',
'feature',
'filters',
'future',
'graph',
'io',
'measure',
'metrics',
'morphology',
'registration',
'restoration',
'segmentation',
'transform',
'util',
'__version__',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is __doc__ going away? I recently read something about __doc__ changing soon, but maybe it was just its parsing...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where __doc__ was previously included? Am I misunderstanding?

Not sure, but I don't think we need to advertise __doc__ in __all__. It's an implementation detail of Python and it's available at runtime for all intents and purposes, e.g., help(skimage).

I just put __version__ in here because it might be useful to advertise.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I wasn't referring to anything 'previously included' or not and, in the context of this PR, I should have!

I was thinking 'in the absolute,' i.e., when I

import skimage as ski

and I run

ski.[tab]

which objects do I expect to be available?

]


# Logic for checking for improper install and importing while in the source
Expand Down Expand Up @@ -117,68 +86,42 @@ def _raise_build_error(e):
import sys

sys.stderr.write('Partial import of skimage during the build process.\n')
# We are not importing the rest of the scikit during the build
# We are not importing the rest of the package during the build
# process, as it may not be compiled yet
else:
try:
from ._shared import geometry

del geometry
except ImportError as e:
_raise_build_error(e)

# Legacy imports into the root namespace; not advertised in __all__
from .util.dtype import (
dtype_limits,
img_as_float32,
img_as_float64,
img_as_float,
img_as_int,
img_as_uint,
img_as_ubyte,
img_as_bool,
)

from .util.lookfor import lookfor

from .data import data_dir


if 'dev' in __version__:
# Append last commit date and hash to dev version information, if available

def _try_append_commit_info(version):
"""Append last commit date and hash to `version`, if available."""
import subprocess
import os.path
from pathlib import Path

try:
p = subprocess.Popen(
output = subprocess.check_output(
['git', 'log', '-1', '--format="%h %aI"'],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
cwd=os.path.dirname(__file__),
cwd=Path(__file__).parent,
text=True,
)
except FileNotFoundError:
if output:
git_hash, git_date = (
output.strip().replace('"', '').split('T')[0].replace('-', '').split()
)
version = '+'.join(
[tag for tag in version.split('+') if not tag.startswith('git')]
)
version += f'+git{git_date}.{git_hash}'

except (FileNotFoundError, subprocess.CalledProcessError):
pass
except OSError:
pass # If skimage is built with emscripten which does not support processes
else:
out, err = p.communicate()
if p.returncode == 0:
git_hash, git_date = (
out.decode('utf-8')
.strip()
.replace('"', '')
.split('T')[0]
.replace('-', '')
.split()
)

__version__ = '+'.join(
[tag for tag in __version__.split('+') if not tag.startswith('git')]
)
__version__ += f'+git{git_date}.{git_hash}'
return version


if 'dev' in __version__:
__version__ = _try_append_commit_info(__version__)


from skimage._shared.tester import PytestTester
from skimage._shared.tester import PytestTester as _PytestTester

test = PytestTester(__name__)
del PytestTester
test = _PytestTester(__name__)
18 changes: 16 additions & 2 deletions skimage/__init__.pyi
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
submodules = [
_submodules = [
'color',
'data',
'draw',
Expand All @@ -18,7 +18,7 @@ submodules = [
'util',
]

__all__ = submodules + ['__version__'] # noqa: F822
__all__ = _submodules + ['__version__'] # noqa: F822

from . import (
color,
Expand All @@ -39,3 +39,17 @@ from . import (
transform,
util,
)

# Legacy import, not advertised in __all__
from .util.dtype import (
dtype_limits,
img_as_float32,
img_as_float64,
img_as_float,
img_as_int,
img_as_uint,
img_as_ubyte,
img_as_bool,
)
from .util.lookfor import lookfor
from .data import data_dir
2 changes: 2 additions & 0 deletions skimage/color/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
"""Color space conversion."""

import lazy_loader as _lazy

__getattr__, __dir__, __all__ = _lazy.attach_stub(__name__, __file__)
3 changes: 1 addition & 2 deletions skimage/data/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""
Test images and datasets.
"""Example images and datasets.

A curated set of general purpose and scientific images used in tests, examples,
and documentation.
Expand Down
2 changes: 2 additions & 0 deletions skimage/draw/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
"""Drawing primitives that operate on NumPy arrays, e.g., lines, text, etc."""

import lazy_loader as _lazy

__getattr__, __dir__, __all__ = _lazy.attach_stub(__name__, __file__)
2 changes: 2 additions & 0 deletions skimage/exposure/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
"""Image intensity adjustment, e.g., histogram equalization, etc."""

import lazy_loader as _lazy

__getattr__, __dir__, __all__ = _lazy.attach_stub(__name__, __file__)
2 changes: 2 additions & 0 deletions skimage/feature/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
"""Feature detection and extraction, e.g., texture analysis, corners, etc."""

import lazy_loader as _lazy

__getattr__, __dir__, __all__ = _lazy.attach_stub(__name__, __file__)
2 changes: 2 additions & 0 deletions skimage/filters/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
"""Sharpening, edge finding, rank filters, thresholding, etc."""

import lazy_loader as _lazy

__getattr__, __dir__, __all__ = _lazy.attach_stub(__name__, __file__)
2 changes: 1 addition & 1 deletion skimage/graph/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
This moddule provides utilities for graph-based image processing.
Graph-based operations, e.g., shortest paths.

This includes creating adjacency graphs of pixels in an image, finding the
central pixel in an image, finding (minimum-cost) paths across pixels, merging
Expand Down
2 changes: 1 addition & 1 deletion skimage/io/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""Utilities to read and write images in various formats."""
"""Reading and saving of images and videos."""

import warnings

Expand Down
2 changes: 2 additions & 0 deletions skimage/measure/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
"""Measurement of image properties, e.g., region properties, contours."""

import lazy_loader as _lazy

__getattr__, __dir__, __all__ = _lazy.attach_stub(__name__, __file__)
2 changes: 2 additions & 0 deletions skimage/metrics/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
"""Metrics corresponding to images, e.g., distance metrics, similarity, etc."""

import lazy_loader as _lazy

__getattr__, __dir__, __all__ = _lazy.attach_stub(__name__, __file__)
8 changes: 1 addition & 7 deletions skimage/morphology/__init__.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
"""Utilities that operate on shapes in images.

These operations are particularly suited for binary images,
although some may be useful for images of other types as well.

Basic morphological operations include dilation and erosion.
"""
"""Morphological algorithms, e.g., closing, opening, skeletonization."""

from .binary import binary_closing, binary_dilation, binary_erosion, binary_opening
from .gray import black_tophat, closing, dilation, erosion, opening, white_tophat
Expand Down
2 changes: 1 addition & 1 deletion skimage/restoration/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""Image restoration module."""
"""Restoration algorithms, e.g., deconvolution algorithms, denoising, etc."""

import lazy_loader as _lazy

Expand Down
2 changes: 1 addition & 1 deletion skimage/segmentation/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""Algorithms to partition images into meaningful regions or boundaries."""
"""Partitioning images into meaningful regions or boundaries."""

from ._expand_labels import expand_labels
from .random_walker_segmentation import random_walker
Expand Down
2 changes: 1 addition & 1 deletion skimage/transform/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""This module includes tools to transform images and volumetric data.
"""Geometric and other transformations, e.g., rotations, Radon transform.

- Geometric transformation:
These transforms change the shape or position of an image.
Expand Down
2 changes: 1 addition & 1 deletion skimage/util/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""General utility functions.
"""Generic utilities.

This module contains a number of utility functions to work with images in general.
"""
Expand Down
Loading