-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
[WIP] MAINT: Wrap printoptions
in ContextVar
#26345
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
e6c3ab5
to
f932b7f
Compare
@@ -1443,10 +1472,11 @@ def _void_scalar_to_string(x, is_repr=True): | |||
scalartypes.c.src code, and is placed here because it uses the elementwise | |||
formatters defined above. | |||
""" | |||
options = _format_options.copy() | |||
options = _format_options.get().copy() |
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 sure it makes sense, but I was half hoping we can just stop passing options around completely and enter a new context ourselves when necessary. contextvar
lookup is almost as fast as dict lookup, and global lookup also not really expensive at the function level (within a loop it might be).
printoptions
in ContextVar
printoptions
in ContextVar
ping @mtsokol |
I'm going to take this over from @mtsokol and also deal with the |
Closing in favor of a new PR closer to the top of the queue. |
This is a re-do of gh-26345, I'm taking over from @mtsokol because this is needed for the free-threaded work. The new _printoptions.py file exists to avoid a circular import during setup of the multiarray module. I'm guessing this adds some overhead to printing. I haven't benchmarked it because it wasn't clear to me: do we care about printing performance? I could certainly add some caching or a way to avoid repeatedly calling get_legacy_print_mode for every printed value. We could also keep the C global we had before but make it thread-local. I just thought it made things conceptually simpler to store all the printoptions state in the context variable. Co-authored-by: Mateusz Sokół <mat646@gmail.com>
This is a re-do of numpygh-26345, I'm taking over from @mtsokol because this is needed for the free-threaded work. The new _printoptions.py file exists to avoid a circular import during setup of the multiarray module. I'm guessing this adds some overhead to printing. I haven't benchmarked it because it wasn't clear to me: do we care about printing performance? I could certainly add some caching or a way to avoid repeatedly calling get_legacy_print_mode for every printed value. We could also keep the C global we had before but make it thread-local. I just thought it made things conceptually simpler to store all the printoptions state in the context variable. Co-authored-by: Mateusz Sokół <mat646@gmail.com>
Issue #26314
Hi @seberg @ngoldbaum,
This (WIP) PR wraps
printoptions
inContextVar
. For now I tried with wrapping whole dict into a one ContextVar to see how the implementation changes - I think I can also clean/refactor the file/functions a bit (e.g.set_string_function
was deprecated in 2.0, maybe we can remove it in 2.1?)