Skip to content

Populate metadata deprecation#5831

Merged
joshmoore merged 3 commits intoome:developfrom
sbesson:populate_metadata_deprecation
Sep 3, 2018
Merged

Populate metadata deprecation#5831
joshmoore merged 3 commits intoome:developfrom
sbesson:populate_metadata_deprecation

Conversation

@sbesson
Copy link
Copy Markdown
Member

@sbesson sbesson commented Aug 22, 2018

This deprecates the metadata modules distributed as part of OMERO.py. The content of these classes has been migrated to https://github.com/ome/omero-metadata available from PyPI and future updates/fixes/improvements will be carried out there.

Upstream tests should remain functional and passing but any usage of the modules e.g. when using the CLI plugin via

OMERO_DEV_PLUGINS=true bin/omero metadata -h

should display the deprecation warning.

See also discussion at ome/omero-parade#47 (comment)

For both the populate_metadata module and the MetadataControl class, add a
DeprecationWarning suggesting to use https://github.com/ome/omero-metadata
instead.
Copy link
Copy Markdown
Member

@manics manics left a comment

Choose a reason for hiding this comment

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

The deprecation warning is printed twice since the warning is in two places, but that's not a big deal.
👍

"This module is deprecated as of OMERO 5.4.8. Use the module"
" in the omero-metadata project available from"
" https://pypi.org/project/omero-metadata/ instead.",
DeprecationWarning)
Copy link
Copy Markdown
Member Author

@sbesson sbesson Aug 28, 2018

Choose a reason for hiding this comment

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

While testing an import, I just realized that this warning is displayed on every import of omero.util.populate_metadata. Concretely that implies it will show up every invocation of the CLI since the first step is to load the plugins even if they are not registered.

Although harmless, my feeling is that this is fairly annoying in terms of user experience. I will look at moving the deprecation warning under the main classes like ParsingContext instead so that the warning happens at run-time and does not create unnecessary noise.

Copy link
Copy Markdown
Member Author

@sbesson sbesson Aug 28, 2018

Choose a reason for hiding this comment

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

Ponderating the comment above, as per https://docs.python.org/2/library/warnings.html#warning-categories, DeprecationWarning is ignored by default on Python 2.7. If we still wanted to reduce the CLI noise on Python 2.6, https://docs.python.org/2/library/warnings.html#available-context-managers would certainly be a way to silence these warnings.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I doubt that Python 2.6 is worth any avoidable effort at all.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps add this:

$ grep warnings bin/omero
import warnings
warnings.filterwarnings("ignore", category=DeprecationWarning)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pushed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thank you

@joshmoore joshmoore added this to the 5.4.8 milestone Sep 3, 2018
@joshmoore joshmoore merged commit 37565d1 into ome:develop Sep 3, 2018
@sbesson sbesson deleted the populate_metadata_deprecation branch September 3, 2018 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants