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

Setup updates 202304 #370

Merged
merged 32 commits into from
Jun 7, 2023

Conversation

mkelley
Copy link
Member

@mkelley mkelley commented Apr 19, 2023

Drop Python 3.7.

numpy -> 1.18
astropy -> 4.3
synphot -> 1.1
astroquery -> 4.6

Update CI scripts.

Drop Python 3.7.

numpy -> 1.18
astropy -> 4.3
synphot -> 1.1
astroquery -> 4.6
alldeps was installing latest versions so oldestdeps were not being tested.
@codecov
Copy link

codecov bot commented Apr 27, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@997d64d). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #370   +/-   ##
=======================================
  Coverage        ?   75.65%           
=======================================
  Files           ?       78           
  Lines           ?     6795           
  Branches        ?        0           
=======================================
  Hits            ?     5141           
  Misses          ?     1654           
  Partials        ?        0           

@mkelley
Copy link
Member Author

mkelley commented Apr 28, 2023

I think this is waiting for old tests to run, but I've replaced those tests. So, ready for review @jianyangli

@mkelley mkelley added this to the v0.4 milestone May 24, 2023
@jianyangli
Copy link
Contributor

@mkelley Can you remind me the motivation for this update? My philosophy is that we want to support the minimum versions of dependence packages that can satisfy the requirements of sbpy, although this philosophy could be too simplified. Do we have to require the upgrade of those packages you changed the version requirement here? Thanks.

@mkelley
Copy link
Member Author

mkelley commented Jun 2, 2023

I should have provided the justification with my PR notes. I understand the need to support a broad set of versions for the users, but that also puts a larger burden on the developers and the testing resources. I recall that we set out to always support astropy's long-term support (LTS) and current versions, which I think are 5.0 and 5.3. However, I don't know where that is documented, and we have to revisit this anyway given that astropy no longer has LTS versions: https://github.com/astropy/astropy-APEs/blob/main/APE21.rst

  • Python 3.7 is no longer supported as of this month: https://www.python.org/downloads/ . I'm a little concerned about supporting a version of python that is no longer getting security fixes.
  • astropy v4.3... I thought this was the LTS version, but now I see it is 5.0. We can leave this as is.
  • synphot 1.1 is needed for compatibility with astropy v4.3
  • numpy 1.18... I'm not sure where I picked that up. Looking at astropy, 4.3 needs numpy >=1.17, astropy 5.0 needs 1.18 <= numpy >=1.25.

So, I have some inconsistencies here to fix. Do recommend we stick with astropy 4.3 and set numpy to 1.17, or move to astropy 5.0?

@mkelley
Copy link
Member Author

mkelley commented Jun 2, 2023

Ah, numpy 1.17 does not support Python 3.8.

And looking at astroquery's release notes, perhaps we can pin that to 0.4.5?

@jianyangli
Copy link
Contributor

Thanks Mike for the detailed rationales! It's great to document it here. I agree with all of them. I think we can stick to astropy 4.3 and astroquery 0.4.5. I don't see strong reasons for lifting the bar further up for these two packages for now.

@mkelley
Copy link
Member Author

mkelley commented Jun 6, 2023

sbpy now requires astroquery>=0.4.5 (down from 0.4.6)

Tests that use from_jplspec now require astroquery 0.4.7, see astropy/astroquery#2717

astropy 5.3 changed how unit strings are represented, which broke some documentation tests. These test now require astropy 5.3, or else they are skipped.

@mkelley
Copy link
Member Author

mkelley commented Jun 7, 2023

@jianyangli finally back to successful builds! Let me know if it is ready to go.

Copy link
Contributor

@jianyangli jianyangli left a comment

Choose a reason for hiding this comment

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

All looks good to me. Thanks for the updates.

sbpy/data/orbit.py Outdated Show resolved Hide resolved
.github/workflows/ci_tests.yml Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the changes in this file are trivial formatting changes and single quotes to double quotes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh crud. This was automatically reformatted with Black. I've managed to avoid that most of the time. Anyway, the important change was in one or two doc tests where I modified 1000 to 1000.0 to get the test to pass.

setup.cfg Show resolved Hide resolved
@mkelley mkelley merged commit 462f453 into NASA-Planetary-Science:main Jun 7, 2023
@mkelley mkelley deleted the setup-updates-202304 branch June 7, 2023 19:34
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.

2 participants