Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Don't fail if we can't query system fonts on macOS#28498
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
The query can fail in some environments where system_profiler is notavailable on PATH, but we don't want it to just crash the entire thing.
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.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join uson gitter for real-time discussion.
For details on testing, writing docs, and our review process, please seethe developer guide
We strive to be a welcoming and open project. Please follow ourCode of Conduct.
Thanks! Seems to make sense to back port. |
I'm not really familiar with the process for that, sorry :( |
Sure, more of a comment to the next reviewer to see if they agree and clarify that I didn't press the 3.9.1 milestone by accident. :-) |
The test failures don't seem related, by the way. |
Agree that the test is probably unrelated, but it is in the same general vicinity so re-running just to be sure. Agree that this should be back-ported. I suspect that we are going to have a second wave of errors with 3.9.1 that are "my fonts are missing" until we understand why what I thought was a macOS system utility is not always available. |
I think you kinda have to explicitly shoot yourself in the foot for this to actually matter, e.g. by unsetting |
#28485 (comment) is also seeing it with conda |
Looks like they're using some sort of a similar build orchestrator, maybe? |
But yeah I think the most basic repro is just |
Something like: is probably what we need to put in CI to make sure we force generating the font cache (rather than loading an existing one). Don't want to hold this PR on it though. |
Temporarily pin an older matplotlib, untilmatplotlib/matplotlib#28498is resolved.Closesidaholab#28045
I don't think this change really has anything to do with when to generate the font cache? Unless there's already an existing one from the previous test runs? |
Temporarily pin an older matplotlib, untilmatplotlib/matplotlib#28498is resolved.Closesidaholab#28045
Temporarily pin an older matplotlib, untilmatplotlib/matplotlib#28498is resolved.Closesidaholab#28045
The mplconfig setting is just to make sure there is no order dependency on when tests are run. At least on GHA we cache this result to save time time on CI. I'm going to go ahead and merge this as-is. I'm getting ideas of a bigger overhaul so we can deal with exhaustive testing with that. |
286e53b intomatplotlib:mainUh oh!
There was an error while loading.Please reload this page.
Thank you for your work on this@K900 and congratulations on your first merged Matplotlib PR 🎉 . I hope we hear from you again. |
Thanks! I hope not, to be honest - I'm more interested in the distro side of things, so no PRs from me would mean things are working as intended, generally :P |
…498-on-v3.9.xBackport PR#28498 on branch v3.9.x (Don't fail if we can't query system fonts on macOS)
PR summary
The query can fail in some environments where system_profiler is not available on PATH, but we don't want it to just crash the entire thing.
PR checklist