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

remove deprecated AsdfFile.open, asdf.open, write_to, update kwargs, #1592

Merged
merged 8 commits into from
Aug 7, 2023

Conversation

braingram
Copy link
Contributor

@braingram braingram commented Jul 17, 2023

With the removal of the deprecated kwargs the **kwargs were also expanded and documented to make it more explicit what arguments are supported.

devdeps will fail due to pyyaml and cython3 incompatibility: yaml/pyyaml#601

stdatamodels downstream failure addressed in: #1594

@braingram braingram added Downstream CI development No backport required labels Jul 17, 2023
@github-actions github-actions bot added this to the 3.0.0 milestone Jul 17, 2023
@braingram braingram changed the title remove deprecated AsdfFile.open remove deprecated AsdfFile.open, asdf.open, write_to, update kwargs, Jul 18, 2023
@braingram braingram marked this pull request as ready for review July 18, 2023 18:24
@braingram braingram requested a review from a team as a code owner July 18, 2023 18:24
)
except Exception:
if close_on_fail:
generic_file.close()
raise

@classmethod
Copy link
Contributor

Choose a reason for hiding this comment

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

Finally!!!!

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a method in this module called test_auto_inline that uses the auto_inline argument, does that test need to go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! Thanks for the catch.

Oddly, this test was never being run :-| It appears that using the exception class in the pytest.mark.filterwarnings decorator causes the test to be skipped (because pytest decorates the exception with the mark then makes the exception with the function as an argument).

I removed the test and it doesn't appear that there is any other use of filterwarnings that provides the exception class as the argument. Thanks again for the catch.

@braingram
Copy link
Contributor Author

The dkist error looks unrelated and appears to be something with astropy trying to download files during test collection:

astropy.utils.iers.iers.IERSWarning: failed to download https://datacenter.iers.org/data/9/finals2000A.all: <urlopen error An attempt was made to connect to the internet by a test that was not marked `remote_data`. The requested host was: datacenter.iers.org>

Copy link
Contributor

@eslavich eslavich left a comment

Choose a reason for hiding this comment

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

LGTM

remove AsdfFile.open mention in docstring
this test was previously never run because of
the odd behavior of pytest when filterwarnings
is provided an exception class (instead of a string).
This results in the exception class being decorated
with the mark, then the function being provided as the
first argument to the class (as the exception class
is used as the decorator). Because the exception class
does not start with "Test" the test is ignored during
collection.
@braingram braingram merged commit ed46538 into asdf-format:main Aug 7, 2023
38 of 40 checks passed
@braingram braingram deleted the asdf_open_args branch August 7, 2023 18:59
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