Skip to content
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

Deprecate skimage.io plugin infrastructure #7353

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

lagru
Copy link
Member

@lagru lagru commented Mar 19, 2024

Description

Still very much a work in progress and trying to understand the problem better. See inline comments below.

See also

Checklist

Release note

We use changelist to
compile each pull request into an item of the release notes. Please refer to
the instructions
and past release notes
for guidance and examples.

...

@lagru lagru added the 📜 type: API Involves API change(s) label Mar 19, 2024
Comment on lines +79 to +84
use_plugin = (
plugin is not DEPRECATED
or plugin_args
or plugin_order()["imread"][0] != "imageio"
)
if use_plugin:
Copy link
Member Author

Choose a reason for hiding this comment

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

It's been proven quite tricky to preserve imread and imsave while deprecating the underlying plugin infrastructure. Honestly, I feel like it's not worth the deprecation headache to keep simple wrappers around but for now I've attempted the following approach:

  • Deprecate usage of the parameters plugin and plugin_args. plugin is also a parameter of imageio which we are planning to wrap. So keeping them around while not silently changing the behavior of user code seems impossible without changing the namespace.
  • If plugin or plugin_args are given or "imageiois not the default plugin (e.g.use_plugin("tifffile")` was used before), use the old plugin infra path. Otherwise simply wrap imageio.

Copy link
Member

Choose a reason for hiding this comment

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

Why not just pop the imageio plugin imread and imwrite into the main module? We do not need to maintain the underlying plugin infrastructure.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because we want a deprecation period during which we support both APIs (our old one and imageio), right?

I'm probably overthinking this. Let me look into this again and try for a simpler approach.



@_deprecate_plugin_function
def imshow(arr, plugin=None, **plugin_args):
Copy link
Member Author

Choose a reason for hiding this comment

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

imshow and similar don't have to be deprecated. I could try to wrap matplotlib in a similar manner to wrapping imageio in im{save,read}. However, before investing the work, I'll first wait if we decide that it's worth keeping them around...



if __doc__ is not None:
__doc__ = _update_doc(__doc__)
__all__ = [
Copy link
Member

Choose a reason for hiding this comment

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

Probably want to hide items from __all__ that are deprecated.

Comment on lines +79 to +84
use_plugin = (
plugin is not DEPRECATED
or plugin_args
or plugin_order()["imread"][0] != "imageio"
)
if use_plugin:
Copy link
Member

Choose a reason for hiding this comment

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

Why not just pop the imageio plugin imread and imwrite into the main module? We do not need to maintain the underlying plugin infrastructure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 type: API Involves API change(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants