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

TST: start testing on Python 3.11 #3956

Merged
merged 2 commits into from
Jul 13, 2022
Merged

Conversation

neutrinoceros
Copy link
Member

PR Summary

Fix #3954
This is expected to fail until upstream is sufficiently stable. Currently the limiting factor is kiwisolver, but it looks like it'll be fixed in the coming days.

@neutrinoceros neutrinoceros added tests: running tests Issues with the test setup enhancement Making something better labels Jun 3, 2022
@neutrinoceros
Copy link
Member Author

A new release of kiwi solver is expected to drop next week (see nucleic/kiwi#142)
I'll rebase and fix the conflict then.

@neutrinoceros neutrinoceros force-pushed the py311_CI branch 2 times, most recently from d56f93c to 93887a9 Compare June 13, 2022 14:56
@neutrinoceros
Copy link
Member Author

Ok here's an actual incompatibility on our side:
yt currently fails to build on CPython 3.11.0b3 with the following error

/opt/hostedtoolcache/Python/3.11.0-beta.3/x64/include/python3.11/pyport.h:61:20: error: ‘nullptr’ was not declared in this scope

This happens because the cykdtree c extension is explicitly requesting -std=c++03, but python/cpython#92253 makes C++11 de facto required (nullptr wasn't a builtin before that)

I'm not sure wether the bumped requirement is intended on the CPython side or not.

@matthewturk
Copy link
Member

So could we bump to c++11 in our build, or remove the line entirely when on python 3.11?

@neutrinoceros
Copy link
Member Author

I think the reason why certain extensions require C++03 is that we wanted to support old compilers like GCC 5 (see #2939)
Ideally we should make this conditional for Python <= 3.10, but I'm don't know how.
Otherwise we may just embrace C++11 as the minimal requirement. It's probably more reasonable now than two years ago anyway, especially since our more rapid release cadence makes it less relevant to install from source.

@matthewturk
Copy link
Member

I have some uncertainty about doing this without checking some supercomputer installations, which are notorious for having out of date compilers. But, we use c++11 for the bitmap index stuff, right?

@neutrinoceros
Copy link
Member Author

Actually we clarified that making C++11 required wasn't intended, and there's a fix in the making python/cpython#93784
So we should be able to compile without any change on our side when this ships !

@neutrinoceros
Copy link
Member Author

After a long delay, Python 3.11.0b4 is available, let's try this again

@neutrinoceros
Copy link
Member Author

not quite. actions/python-versions#177 is still pending

@neutrinoceros
Copy link
Member Author

It compiles !
Now we're seeing 4 occurrences of the same problem, for instance:

DeprecationWarning: It is deprecated to return a value!=None from a test case (<bound method SimpleVRTest.test_simple_vr of <yt.visualization.volume_rendering.tests.test_simple_vr.SimpleVRTest testMethod=test_simple_vr>>)

This seems to be a pytest deprecation. I don't understand why it's only showing up now but a quick inspection seems to indicate that in all 4 cases, the return value of these tests are actually not used, so it shouldn't hurt to just remove them.

@neutrinoceros neutrinoceros marked this pull request as ready for review July 13, 2022 10:49
@neutrinoceros
Copy link
Member Author

This is now ready !

@matthewturk can you take a look ? The sooner this gets in the sooner we'll be able to report any problem with 3.11

@@ -66,4 +66,3 @@ def test_points_vr(self):
sc.add_source(points_source)
im = sc.render()
im.write_png("points.png")
return im
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I thought for a minute that removing the return would make this harder to debug, but now I'm not sure that it matters. So I'm OK with this.

@matthewturk matthewturk enabled auto-merge July 13, 2022 10:54
@matthewturk matthewturk merged commit aeb4d05 into yt-project:main Jul 13, 2022
@neutrinoceros neutrinoceros deleted the py311_CI branch July 13, 2022 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Making something better tests: running tests Issues with the test setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: use stdlib tomllib when available (Python 3.11)
2 participants