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.

Sign up for GitHub

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

Jump to bottom

Clean up test dependencies, add python 3.12 testing #1633

Merged
merged 6 commits into from
Sep 29, 2023
Merged

Clean up test dependencies, add python 3.12 testing #1633

merged 6 commits into from
Sep 29, 2023

Conversation

braingram
Copy link
Contributor

@braingram braingram commented Aug 18, 2023 •
edited
Loading

After #1594 is sorted (which at the moment removes the gwcs test dependency) this PR can be finished and brought out of draft.

EDIT: based off changes in #1650 which fixes 1 test for python 3.12.

The changes in this PR clean up the test dependencies. Some likely targets are:

  • gwcs: looks to be entirely unused
  • asdf-astropy: used for a handful of outdated tests and for some tests of 'nested' objects which could be handled by custom objects
  • astropy: mostly incorrect usage now that serialization has moved to asdf-astropy

This also adds 3.12 testing which will currently fail because of aiohttp (required by fsspec). See #1651 for results with fsspec removed as a test dependency.

This also re-adds pyyaml to devdeps testing as the cython issues appear to be addressed:
Fixes: #1607

braingram
braingram commented Aug 18, 2023
View reviewed changes
asdf/_tests/test_api.py
@@ -119,23 +118,6 @@ def test_atomic_write(tmp_path, small_tree):
ff.write_to(tmpfile)


def test_overwrite(tmp_path):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue mentioned for this test:
#100

describes that it was added to try and replicate an issue on windows. The test did not replicate the issue: #302 (comment)
yet the test was left in place.

@braingram braingram force-pushed the cleanup_deps branch 4 times, most recently from 8304a7c to 740b7b4 Compare August 18, 2023 18:05
@braingram braingram mentioned this pull request Aug 18, 2023
Merged
@braingram braingram changed the title Cleanup deps TEST: Cleanup deps Aug 18, 2023
@braingram braingram force-pushed the cleanup_deps branch 3 times, most recently from a308eb4 to cc66fbf Compare August 21, 2023 14:17
@braingram braingram mentioned this pull request Aug 21, 2023
Closed
@braingram braingram force-pushed the cleanup_deps branch 2 times, most recently from 7404ae3 to e1ce3f7 Compare September 8, 2023 19:01
@braingram braingram mentioned this pull request Sep 8, 2023
Merged
@braingram braingram added this to the 3.0.0 milestone Sep 11, 2023
@braingram braingram force-pushed the cleanup_deps branch 2 times, most recently from 928f1ae to 2992645 Compare September 18, 2023 12:10
braingram
braingram commented Sep 18, 2023
View reviewed changes
asdf/_tests/test_generic_io.py Outdated
@@ -394,7 +393,7 @@ def test_invalid_obj(tmp_path):


def test_nonseekable_file(tmp_path):
base = io.IOBase
base = io.FileIO
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why IOBase wasn't failing with 3.12.rc1 but it fails with 3.12.dev. I don't see any description of this change in the release notes or PRs. My suspicion is the use of IOBase here was working in part because of a bug that is fixed in 3.12 (as providing a filename to IOBase.__init__ doesn't really make sense so I could see how this could be fixed as bug during related io cleanup).

@braingram braingram mentioned this pull request Sep 21, 2023
Merged
@braingram braingram mentioned this pull request Sep 22, 2023
Closed
@braingram
Copy link
Contributor Author

stdatamodels CI failing due to crds context update. This should (at least partially) be addressed in: spacetelescope/stdatamodels#219
I also see dynamicdq warnings but these also appear in stdatamodels main:
https://github.com/spacetelescope/stdatamodels/actions/runs/6265797233/job/17015381989

@braingram braingram changed the title TEST: Cleanup deps Clean up test dependencies, add python 3.12 testing Sep 22, 2023
@braingram braingram marked this pull request as ready for review September 29, 2023 13:30
@braingram braingram requested a review from a team as a code owner September 29, 2023 13:30
eslavich
eslavich approved these changes Sep 29, 2023
View reviewed changes
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.

🚀

@braingram
Copy link
Contributor Author

Now if only RTD would keep up with what appears to be a caffeinated Slavich.

@braingram braingram merged commit ca63097 into asdf-format:main Sep 29, 2023
40 of 43 checks passed
@braingram braingram deleted the cleanup_deps branch September 29, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eslavich eslavich eslavich approved these changes

Assignees
No one assigned
Labels
Downstream CI no-changelog-entry-needed
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.

Re-add pyyaml to devdeps testing when it gets cython 3 compatibility
3 participants
@braingram @eslavich @4sk13