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

Add back support for python 3.9 for numpy 2 compatibility woes #7412

Merged
merged 3 commits into from
Jun 3, 2024

Conversation

hmaarrfk
Copy link
Member

@hmaarrfk hmaarrfk commented May 7, 2024

It seems highly probable that numpy 2 will release a build for Python 3.9
@rgommers had asked us to add an upper bound to our releases in Jan but it seems that we didn't take action (understanding how pip solves packages is unclear to me even).

Instead of releasing 0.22.1 for python 3.9 with metadata updates, i'm curious to see if we can release a new version with python 3.9.

The headache of getting CIs working is what I'm hoping to avoid with this startegy

xref: https://mail.python.org/archives/list/[email protected]/thread/AHTATJKGUEOILBNUI5IGGZPXJ5FXIRAU/
xref: #7282 (comment)

  • Make a PR to scikit-image-build

Checklist

Release note

For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.

Add support back for Python 3.9 to enhance compatibility with Numpy 2.

@hmaarrfk hmaarrfk added the 🔧 type: Maintenance Refactoring and maintenance of internals label May 7, 2024
@hmaarrfk
Copy link
Member Author

hmaarrfk commented May 7, 2024

i expected some tests to fail, but I did not expect them to catastrophically fail like this on CPython 3.9

@hmaarrfk hmaarrfk force-pushed the support_python3.9 branch 3 times, most recently from d8973a3 to 41e271e Compare May 7, 2024 13:18
@@ -747,10 +747,11 @@ def test_inverse_all_transforms(tform):
assert_almost_equal(tform.inverse.inverse(SRC), tform(SRC))
# Test addition with inverse, not implemented for all
if not isinstance(
tform,
EssentialMatrixTransform
| FundamentalMatrixTransform
Copy link
Member Author

Choose a reason for hiding this comment

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

man, this syntax is really strange...

Copy link
Member Author

Choose a reason for hiding this comment

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

FYI it seems that this was added by some autofromatting code

d93ce53

@jarrodmillman is there a way for me to disable this kind of syntax reformatting?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

See #7430.

@hmaarrfk hmaarrfk marked this pull request as ready for review May 7, 2024 14:14
@hmaarrfk
Copy link
Member Author

hmaarrfk commented May 7, 2024

I think this is ready again for review.

While I was very hopeful that NEP29/SPEC0 could be adhered to, I feel like we might need to rethink the general strategy of:

"Does numpy continue support the longest"
or
"Is numpy the first to drop support for older pythons"

Given the likely release of Numpy2 for Python 3.9, I think we should continue to support it, at least until the major bugs have been caught with it.

@lagru
Copy link
Member

lagru commented May 7, 2024

I'm personally not opposed to making a last release to support 3.9. 👍

@lagru
Copy link
Member

lagru commented May 7, 2024

But I'd probably prefer a patch release 0.23.3 with the relevant commits cherry-picked. I'm not sure what you are suggesting as a new version otherwise in

i'm curious to see if we can release a new version with python 3.9.

@hmaarrfk
Copy link
Member Author

hmaarrfk commented May 7, 2024

There are two options in my mind:

  1. Going back to 0.22.1 + python 3.9
  2. Releasing 0.23.3 + python 3.9

I'm providing the option for 0.23.3 since I think that getting the CIs working for 0.22.X will be hard. The work on the LTS still haunts me.

It seems highly probable that numpy 2 will release a build for
Python 3.9
@rgommers had asked us to add an upper bound to our releases in Jan but it
seems that we didn't take action (understanding how pip solves packages
is unclear to me even).

Instead of releasing 0.22.1 for python 3.9 with metadata updates, i'm
curious to see if we can release a new version with python 3.9.

The headache of getting CIs working is what I'm hoping to avoid with
this startegy

xref: https://mail.python.org/archives/list/[email protected]/thread/AHTATJKGUEOILBNUI5IGGZPXJ5FXIRAU/
@hmaarrfk
Copy link
Member Author

The CIs are green. Any interest in moving on this?

@stefanv
Copy link
Member

stefanv commented May 13, 2024

I'd like to have @jarrodmillman weigh in.

@hmaarrfk
Copy link
Member Author

it would be good to get a move on this if there is desire.

We are quite good at releasing quality packages, I think this will help many build confidence in the python ecosystem if numpy 2 doesn't break their pipelines.

Copy link
Member

@mkcor mkcor 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 very much in favour of this because:

We are quite good at releasing quality packages, I think this will help many build
confidence in the python ecosystem if numpy 2 doesn't break their pipelines.

👍

But @jarrodmillman is the expert! 🙇

For my own understanding, @hmaarrfk, what does

"Does numpy continue support the longest"

mean? I get the question

"Is numpy the first to drop support for older pythons"

and it would be good to know if that's an absolute "yes."

skimage/feature/_fisher_vector.py Outdated Show resolved Hide resolved
@hmaarrfk
Copy link
Member Author

hmaarrfk commented Jun 3, 2024

"Does numpy continue support the longest"

should be

"Does numpy continue support python's the longest"

@jarrodmillman
Copy link
Contributor

+1 from me. I will take a closer look at this today (at the Scientific Python Developer Summit!) and get something out soon.

@hmaarrfk hmaarrfk merged commit 0fa79b2 into scikit-image:main Jun 3, 2024
25 checks passed
@stefanv stefanv added this to the 0.24 milestone Jun 3, 2024
@mkcor
Copy link
Member

mkcor commented Jun 8, 2024

Does anyone know why we have

"tomli; python_version < '3.11'",

? Just noticed:

$ pip install -r requirements.txt
Ignoring tomli: markers 'python_version < "3.11"' don't match your environment
(...)

while setting up my dev env in Python 3.11.3!

@neutrinoceros
Copy link

@mkcor tomli was added to the standard library (as tomllib) in Python 3.11.0, so it's normally not used as a dependency if you're running on 3.11 or newer.

@mkcor
Copy link
Member

mkcor commented Jun 10, 2024

@mkcor tomli was added to the standard library (as tomllib) in Python 3.11.0, so it's normally not used as a dependency if you're running on 3.11 or newer.

Thank you, @neutrinoceros, good to know! I didn't know how to interpret that tomli only supported Python from 3.7 through 3.10 (https://pypi.org/project/tomli/).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔧 type: Maintenance Refactoring and maintenance of internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants