Skip to content

Conversation

@jeremyh
Copy link
Contributor

@jeremyh jeremyh commented Aug 27, 2021

This PR may be controversial, but I noticed our Docker container now uses Python 3.8, and hence all of our tests are now only running on Python 3.8 (from what I can see?).

  • (Edit: there's also a workflow I missed that does a conda install to Python 3.7, but it doesn't run the test suite.)

My experience is that it's very easy to break 3.6 support by accident if not testing with it. And we're increasingly seeing dependencies drop support for 3.6 on other repositories in the ODC ecosystem.

Some options:

  1. A recommendation of 3.8. The first commit updates all docs to recommend and default to Python 3.8, but setup.py still allows Python 3.6 installs.
  2. A hard requirement on 3.8: the second commit requires Python 3.8+ in setup.py too, so users cannot accidentally pip install newer versions on 3.6. (edit: and to bump conda smoke test version)

I'm happy to remove the second commit if people think it's too harsh. But I think a minimum requirement is friendlier to users than exposing people to unexpected bugs.

Thoughts?

ps. The other option, of course, it to make 3.7 the minimum. But is that still common anywhere, among non-Docker users? And do we want to test with it?
pps. I also bumped the Postgres version recommendation, since none of us test on 9.5. But there's no hard requirement.

... since that is what all of our systems use and test with now.
... as that is what all systems and tests run on now.
@snowman2
Copy link
Contributor

For reference on making Python 3.7 minimum supported version: pyproj4/pyproj#790

@codecov
Copy link

codecov bot commented Aug 27, 2021

Codecov Report

Merging #1190 (96bcddc) into develop (3a49f78) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1190   +/-   ##
========================================
  Coverage    93.75%   93.75%           
========================================
  Files          102      102           
  Lines        10352    10352           
========================================
  Hits          9705     9705           
  Misses         647      647           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a49f78...96bcddc. Read the comment docs.

Copy link
Member

@Kirill888 Kirill888 left a comment

Choose a reason for hiding this comment

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

I have no issues with this change.

We currently run tests with 3.8 only, but I guess we "test" 3.6 in sandbox, which will hopefully soon switch to 3.8 too.

Copy link
Member

@omad omad left a comment

Choose a reason for hiding this comment

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

I'm fine with deprecating 3.6 support and skipping 3.7.

NCI and DEA Sandbox are both still running 3.6, but are both long overdue for an update.

@omad
Copy link
Member

omad commented Aug 27, 2021

It's also good timing soon after a release. We should bump to 1.9.0 for the next release.

@Kirill888
Copy link
Member

@woodcockr EASI is using Python 3.8 already isn't?

@alexgleith
Copy link
Contributor

alexgleith commented Aug 27, 2021

Sandbox images are now working in DE Africa with Python 3.8 and it seems fine.

@jeremyh
Copy link
Contributor Author

jeremyh commented Aug 27, 2021

NCI and DEA Sandbox are both still running 3.6, but are both long overdue for an update.

I think part of the previous difficulty was because the DEA code had a dependency that was strictly python3.6-only. I have a PR to move it: GeoscienceAustralia/digitalearthau#305

@woodcockr
Copy link
Member

@Kirill888 Yep, EASI is on Python 3.8 already and everything we use it working fine. Thank you for asking.

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.

7 participants