-
-
Notifications
You must be signed in to change notification settings - Fork 8k
ENH: Gracefully handle python-build-standalone ImportError with Tk #30394
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
Open
timhoffm
wants to merge
6
commits into
matplotlib:main
Choose a base branch
from
timhoffm:python-build-standalone-importerror
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
64d0b0c
ENH: Gracefully handle python-build-standalone ImportError with Tk.
timhoffm bc2ed87
DOC: add canonical uv update command in error message
tacaswell 59b29f5
DOC: tweak suggestion for upgrading uv managed Python
tacaswell c332377
Fix line length
timhoffm ed12aa9
Mentioned fix in recent python-build-standalone
timhoffm 6d1f6d3
whitespace fix
timhoffm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
According to astral-sh/python-build-standalone#382, you might be able to use what
uv
uses: https://github.com/astral-sh/uv/blob/c25c800367a5f43069f1c9d778cfd5de1bcc54a6/crates/uv-python/python/get_interpreter_info.py#L639-L645 (i.e.,sysconfig
values).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.
Is that more helpful?
I think we should catch the error, not exclude python-build-standalone per-se. If they'd fix the tk issue, our versions would directly work. On
is_uv_python
- I've explicitly tested for uv and tailored the error message, because I anticipate that'll by far the most route to trigger this, and I project that most uv users will not know what python-build-standalone is. So let's give them a uv-targeted error message.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.
I didn't suggest excluding python-build-standalone, just detecting it differently.
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.
My point is I do not really care about python-build-standalone. The criterion is 1. does it raise the known error? And if so, is it a uv-install that I can point users to as the practical cause.
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 that's the case that you only care about
uv
, then the PR/commit title should be adjusted.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.
Possibly my wording was not exactly precise.
The issue originates from the current way python-build-standalone is implmented. - So the PR title is justified.
Then there are two aspects to handling:
Detection: I want to detect the specific error and respond to that. - It's not helpful to detect python-build-standalone. They may improve to fix the error in the future. So issuing a warning/error purely on the fact that python-build-standalone is used would be presumptuous.
Error message: In the simplest case, we could just have the second error message
(or if you want to be more affirmative explicitly test for python-build-standalone and remove the "likely"). However, the by-far most common way to obtain a python-build-standalone python is via
uv
. But it's a technical detail ofuv
and users are most likely not aware of it. So telling them, they have python-build-standalone is not helpful for fixing the issue. Therefore, I added theuv
detection to tell them that uv-provided python is not compatible.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.
I am 👍 on doing the work to detect if it is uv.