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

Bumped minimum version to 3.8 #1163

Merged
merged 6 commits into from
Aug 14, 2024
Merged

Bumped minimum version to 3.8 #1163

merged 6 commits into from
Aug 14, 2024

Conversation

tabedzki
Copy link
Contributor

To close #1162
Happy to make other contributions as necessary to upgrade the minimum version.

@tabedzki
Copy link
Contributor Author

@dimitri-yatsenko @ethho Can we update the Python versioning to drop support for end-of-life versions of Python (3.7)?

@ethho ethho requested review from dimitri-yatsenko and ethho July 29, 2024 14:16
@ethho
Copy link
Contributor

ethho commented Jul 29, 2024

Thanks for the PR @tabedzki! Agreed that we should drop 3.7 in favor of 3.12.

Can you also change the CI so that we test Python3.12 + MySQL 8.0, and drop the 3.7 tests? I can't make the change on your fork:

include:
- py_ver: "3.11"
mysql_ver: "8.0"
- py_ver: "3.10"
mysql_ver: "8.0"
- py_ver: "3.8"
mysql_ver: "5.7"
- py_ver: "3.7"
mysql_ver: "5.7"

EDIT: Seems that https://github.com/datajoint/datajoint-python/actions/runs/9976604874/job/28053793748?pr=1163 is failing for 3.7 because of this change, as expected. Let's remove 3.7 from the PY_VER matrix.

@dimitri-yatsenko
Copy link
Member

Also please update the CHANGELOG.md

@tabedzki
Copy link
Contributor Author

Done! Let me know if there are any other changes I should make.
Also, will the pip package automatically update after this PR or is there a manual step involved?

@ethho
Copy link
Contributor

ethho commented Jul 29, 2024

CI job failed because there is no Docker image datajoint/djtest:py3.12-alpine.

We need to add 3.12 and build the image from the datajoint/djbase-docker repository (has CI, looks like it's never been used), similar to the addition of 3.11 in this PR.

EDIT: just saw datajoint/djtest-docker#7, running the build now.

@ethho ethho self-assigned this Jul 29, 2024
@ethho
Copy link
Contributor

ethho commented Jul 29, 2024

Depends on datajoint/djbase-docker#36

@ethho
Copy link
Contributor

ethho commented Jul 29, 2024

To summarize: we need to add python 3.12 to every image in the dependency tree, in this order:

@tabedzki
Copy link
Contributor Author

@ethho Since Python 3.13 will be released on October 1st of this year, is there any prep work we can do now or should this be broached in October? Python 3.8 also goes end of life that same month.

Python support schedule

@ethho
Copy link
Contributor

ethho commented Jul 29, 2024

Since Python 3.13 will be released on October 1st of this year, is there any prep work we can do now or should this be broached in October

@tabedzki We install Python in the image via conda, so let's wait until a stable 3.13 release is available via conda channels before updating.

This whole dependency tree of Docker images is cumbersome; we should simplify it by maintaining a minimal set of images either in this repository or datajoint/djbase.

@tabedzki
Copy link
Contributor Author

tabedzki commented Aug 1, 2024

@ethho

I took a look at the datajoint/miniconda3-docker and datajoint/dj-base repositories to get an understanding of the workflow. I saw that the code is tested on both alpine and distros and packaged with conda.

  • Since conda is used to install python, would you be open to using the github action setup-python?
  • Also, would you be open to using the github action setup alpine as an alternative to the alpine docker images?

Please let me know if there are any consideration details that I have may missed in these suggestions (i.e., if conda installs more than just python/if docker images are used elsewhere for testing and debugging purposes).

@ethho
Copy link
Contributor

ethho commented Aug 1, 2024

  • Since conda is used to install python, would you be open to using the github action setup-python?

@tabedzki AFAIK, setup-python GHA cannot be used to set up python in a Docker context, which is what we need here. I've also never used it with conda before.

if docker images are used elsewhere for testing and debugging purposes

You are correct. Unfortunately, images such as datajoint/djbase and datajoint/miniconda3 are used in many of the DataJoint codebases, which is primarily why unwinding this tech debt is expensive.

IMO the best approach now is to manually build a 3.12 miniconda image, with the Dockerfile (pseudocode):

FROM datajoint/miniconda3:py312
RUN conda install python=3.12

It doesn't make sense to maintain separate miniconda images for different python versions anyway.

@tabedzki
Copy link
Contributor Author

tabedzki commented Aug 1, 2024

@ethho thanks for giving me a fuller picture. I didn't realize how heavily dependent the system was on those docker images. Therefore, moving away from the conda installation and using the github actions, in the current setup, doesn't make sense.

Looking forward to the Python 3.12 support when it happens. Thank you for your effort on this.

@tabedzki
Copy link
Contributor Author

tabedzki commented Aug 2, 2024

@ethho since there is more work to add 3.12 than there is to remove 3.7 support, I've removed aspects of the PR that would be more burdensome and only focused on removing things that tied the repo to <3.8 in order to merge this request.

Additionally, I've updated the tags used for a couple of github actions since I noticed they were out of the date.

I can open a separate issue that PR that would focus on 3.12 support.

CHANGELOG.md Outdated Show resolved Hide resolved
@ethho
Copy link
Contributor

ethho commented Aug 6, 2024

I can open a separate issue that PR that would focus on 3.12 support.

@tabedzki Please do make a separate issue for this, thanks! The docker builds need some work; I'll make another ticket to remove dependency on these images as noted in my previous comment.

CI runs such as this one now failing due to use of docker-compose instead of subcommand docker compose. Plan to fix in #1164, merge to master, then merge master to this branch.

@tabedzki
Copy link
Contributor Author

tabedzki commented Aug 7, 2024

@ethho I just learned about SPEC 0 — Minimum Supported Dependencies, a coordinated effort endorsed by the major scientific packages (SciPy, NumPy, scikit-image, matplotlib, etc) to adopt a common time-based policy for dropping dependencies after 36 months from the feature release (e.g., 3.9.0, not 3.9.x). Based off this recommendations, the oldest Python version that should be supported is Python 3.10, which would be dropped according to SPEC 0.

Likewise, numpy itself has officially dropped support for Python 3.9 as noted here and SciPy takes guidance from Numpy's schedule:

On Apr 05, 2024 drop support for Python 3.9 (initially released on Oct 05, 2020)
On Jun 22, 2024 drop support for NumPy 1.23 (initially released on Jun 22, 2022)

As we are already updating which versions of Python datajoint supports, I think we should consider which version of Python datajoint officially support. If the minimum version was updated to Python 3.10, there would be fewer versions to support and hence less testing and possible better coordination between package dependencies.

@tabedzki
Copy link
Contributor Author

@ethho and @dimitri-yatsenko What are your thoughts on adopting the same support timetable as other scientific Python packages (e.g., numpy, scipy, etc), or adopting the timetable from Scientific Python?

  • Numpy/Scipy's current timetable:
    • Now: Bump minimum supported version to 3.10
    • Apr 04, 2025: Bump minimum supported version to 3.11
  • Scientific Python's current timetable:
    • Now: Bump minimum supported version to 3.10
    • 2024 - Quarter 4: Bump minimum supported version to 3.11

@ethho
Copy link
Contributor

ethho commented Aug 13, 2024

What are your thoughts on adopting the same support timetable as other scientific Python packages (e.g., numpy, scipy, etc), or adopting the timetable from Scientific Python?

@tabedzki Thanks for bringing this to our attention. I see the motivation here: SciPy has historically had a tough time supporting many Python versions.

In my opinion, datajoint should continue to support older versions of Python as long as the maintenance cost is minimal. We should deprecate older versions on a case-by-case basis, when we encounter instances where the maintenance cost is non-trivial. This ticket is one example; it seems to be caused by a breaking networkx change.

The reasoning is that datajoint occupies a different space in the scientific Python stack than packages like numpy, scipy, and pandas. New features in numpy / scipy are tightly coupled to Python version and target arch. datajoint is not tightly coupled to Python version, and therefore the maintenance cost of supporting legacy versions is relatively small. In other words, new datajoint features are unlikely to use specific Python language features. datajoint is, however, tightly coupled to the database backend. This is why we adopted this approach and deprecated MySQL 5.7 and only test for MySQL 8.

cc: @ttngu207 @dimitri-yatsenko

@tabedzki
Copy link
Contributor Author

@ethho thank you for the in-depth explanation. I can understand and appreciate the reasoning.

@ethho and @dimitri-yatsenko
Since I refocused this PR to just bumping to 3.8 by removing references to Python 3.12, and because this PR passes all the current CI checks, can we merge this PR?

@ethho ethho merged commit 701bb0b into datajoint:master Aug 14, 2024
11 checks passed
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.

Remove Python 3.7 support
3 participants