-
Notifications
You must be signed in to change notification settings - Fork 891
attempt to improve unsupported Python version error #5519
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
Conversation
|
I personally find the idea of hitting the network to check versions somewhat distasteful :-/ |
|
Fair enough, I pushed a second commit which removed the internet query and just attempts to improve the message slightly. |
| the configured Python version (3.13) is newer than PyO3's maximum supported version (3.12)\n\ | ||
| = help: this package is being built with PyO3 version ", env!("CARGO_PKG_VERSION"), "\n\ | ||
| = help: check https://crates.io/crates/pyo3 for the latest PyO3 version available\n\ | ||
| = help: updating this package to the latest version of PyO3 may provide compatibility with this Python version\n\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If anyone has got a moment to review / comment on this message, this is the last small tweak I'd like to merge into 0.27.
(I think the existing error message has been a frequent source of confusion for users adopting 3.14, maybe this new text is slightly better?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a user who admittedly gets confused by pyo3 from time to time, I feel the new error message is indeed a minor improvement :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reads fine to me and is a bit more explicit than before. I guess we can only wait and see if it helps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I agree with alex that we should preferably not start querying the network here.
| /// | ||
| /// This must be called from PyO3's build script, because it relies on environment | ||
| /// variables such as `CARGO_CFG_TARGET_OS` which aren't available at any other time. | ||
| #[allow(dead_code)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not actionable now, but after #5486 we can make use of #[expect(...)] for these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, it would be nice to apply across the codebase
| the configured Python version (3.13) is newer than PyO3's maximum supported version (3.12)\n\ | ||
| = help: this package is being built with PyO3 version ", env!("CARGO_PKG_VERSION"), "\n\ | ||
| = help: check https://crates.io/crates/pyo3 for the latest PyO3 version available\n\ | ||
| = help: updating this package to the latest version of PyO3 may provide compatibility with this Python version\n\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reads fine to me and is a bit more explicit than before. I guess we can only wait and see if it helps.
|
Thanks for the review! |
This PR attempts to improve the situation in #5505 by using the Python interpreter to check what the latest PyO3 version is.
See the test for example output.