Skip to content

Make RC configurator thread local #276

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

Draft
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

cvanelteren
Copy link
Contributor

Attempt to fix the discrepancies between the tests by putting a read lock on the config properties.

@cvanelteren
Copy link
Contributor Author

cvanelteren commented Jun 19, 2025

TODO: replace with copy-on-write

  • added thread local rc params

@cvanelteren cvanelteren marked this pull request as draft June 19, 2025 16:38
Copy link

codecov bot commented Jun 19, 2025

Codecov Report

Attention: Patch coverage is 71.73913% with 52 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
ultraplot/config.py 69.07% 35 Missing and 12 partials ⚠️
ultraplot/internals/rcsetup.py 66.66% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

@cvanelteren cvanelteren changed the title Read lock on configurator Make RC configurator threadlocal Jun 19, 2025
@cvanelteren cvanelteren changed the title Make RC configurator threadlocal Make RC configurator thread local Jun 19, 2025
@cvanelteren
Copy link
Contributor Author

Not sure if I am making the situation worse. The issue is I think generated by threads modifying rc values which causes issues with other tests.

The tests that we were initially failing are in particular moddifying the rc and causing issues. Will need to return to this as this has high priority now.

@beckermr
Copy link
Collaborator

I suspect that matplotlib itself would need to be thread safe in order for this to work. My guess is that it is not.

Maybe try an approach where we force a small subset of the tests to be done serially?

@cvanelteren
Copy link
Contributor Author

That could indeed be the case, but I think the edits all go through this configurator object. This would mean that if we protect it, the matplotlib rc file would be protected too. I will go back now and check the thread safety as I was a bit hazzy when I looked at this last night.

@cvanelteren cvanelteren force-pushed the hotfix-thread-safety-config-read branch from ed13154 to 5129f15 Compare June 20, 2025 12:22
@cvanelteren
Copy link
Contributor Author

I need to separate the initialising of the colormaps before this can be fixed more to follow soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants