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 AsdfFile.extensions and the 'extensions' argument to AsdfFile and asdf.open #1476

Conversation

eslavich
Copy link
Contributor

The AsdfConfig object provides a way to add extensions at runtime, so we don't need to continue to allow custom extensions on the AsdfFile.

@eslavich eslavich requested a review from a team as a code owner March 13, 2023 07:04
@github-actions github-actions bot added this to the 2.15.0 milestone Mar 13, 2023
pyproject.toml Outdated Show resolved Hide resolved
@eslavich eslavich added development No backport required and removed backport-2.15.x labels Mar 13, 2023
@eslavich eslavich modified the milestones: 2.15.0, 3.0.0 Mar 13, 2023
Copy link
Contributor

@braingram braingram left a comment

Choose a reason for hiding this comment

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

Overall looks good. I made some minor warning message suggestions (adding what should be a link to the docs).

I also scanned the downstream run logs for more AsdfDeprecationWarning messages and don't see any new ones (there are some in WeldX from other deprecations which they said they'll fix once 2.15 is released and some in JWST because of the unreleased changes in stpipe). Are there any uses for these arguments/methods that you're aware of?

@@ -69,6 +69,7 @@ def __init__(
if possible and if created from `asdf.AsdfFile.open`.

extensions : object, optional
DEPRECATED
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a concise link we can add here that will render in the docs, something like (I'm not sure if this will render right so the docs should be checked if this suggestion is used).

Suggested change
DEPRECATED
DEPRECATED See :ref:`extending_extensions_installing_asdf_config`

Comment on lines +125 to +127
"The 'extensions' argument to AsdfFile is deprecated. Use asdf.get_config().add_extension to "
"add an extension at runtime.",
AsdfDeprecationWarning,
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the other suggestion:

Suggested change
"The 'extensions' argument to AsdfFile is deprecated. Use asdf.get_config().add_extension to "
"add an extension at runtime.",
AsdfDeprecationWarning,
"The 'extensions' argument to AsdfFile is deprecated. Please see :ref:`extending_extensions_installing_asdf_config`.",
AsdfDeprecationWarning,

Comment on lines +238 to +240
"AsdfFile.extensions is deprecated. Use asdf.get_config().extensions to "
"get a list of enabled extensions.",
AsdfDeprecationWarning,
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to other suggestions.

Suggested change
"AsdfFile.extensions is deprecated. Use asdf.get_config().extensions to "
"get a list of enabled extensions.",
AsdfDeprecationWarning,
"AsdfFile.extensions is deprecated. Please see :ref:`extending_extensions_installing_asdf_config`.",
AsdfDeprecationWarning,

Comment on lines +259 to +261
"AsdfFile.extensions is deprecated. Use asdf.get_config().add_extension to "
"add an extension at runtime.",
AsdfDeprecationWarning,
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Suggested change
"AsdfFile.extensions is deprecated. Use asdf.get_config().add_extension to "
"add an extension at runtime.",
AsdfDeprecationWarning,
"AsdfFile.extensions is deprecated. Please see :ref:`extending_extensions_installing_asdf_config`.",
AsdfDeprecationWarning,

Comment on lines +1895 to +1897
"The 'extensions' argument to asdf.open is deprecated. Use asdf.get_config().add_extension to "
"add an extension at runtime.",
AsdfDeprecationWarning,
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Suggested change
"The 'extensions' argument to asdf.open is deprecated. Use asdf.get_config().add_extension to "
"add an extension at runtime.",
AsdfDeprecationWarning,
"The 'extensions' argument to asdf.open is deprecated. Please see :ref:`extending_extensions_installing_asdf_config`.",
AsdfDeprecationWarning,

@eslavich
Copy link
Contributor Author

eslavich commented Aug 1, 2023

This branch is also decaying and I don't see a clear justification for rushing to include it in the 3.0 release. I'll close the PR and we can open a new one later if we decide this is a worthy effort.

@eslavich eslavich closed this Aug 1, 2023
@eslavich eslavich deleted the eslavich-deprecate-asdf-file-extensions branch August 1, 2023 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development No backport required Downstream CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants