-
Notifications
You must be signed in to change notification settings - Fork 7
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
Rename WaveorderReader
to ImageReader
#46
Conversation
WaveorderReader
to ImageReader
WaveorderReader
to ImageReader
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks @ziw-liu.
I see that ImageReader
still uses the (soon to be deprecated) ZarrReader
. Can I suggest a deprecation warning on ZarrReader
that points users towards the new open_ome_zarr
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
happy to see this renaming @ziw-liu , I also second the deprecation suggestion from @talonchandler regarding ZarrReader
Thanks for the deprecation warning, @ziw-liu. What do you think about reducing the default I think it's really important to alert users that are currently using |
I just realized that I'm not using that custom logger in other parts of the code. So despite being a sub-optimal state the deprecation warning does show up if you invoked it in |
If I run this script:
with
|
Thanks for testing this! I tested in ipython and assumed the same behavior for a script. Then we do need to change log level in this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fixes @ziw-liu!
This PR does a shallow renaming: the legacy
WaveorderReader
class is nowImageReader
. Otherwise API remains the same.