MAINT: back printoptions with a true context variable #26846
Conversation
|
I was wondering if we should stop passing around Unlike the old code, contextvars should protect us from having to "freeze" the state early on to avoid threading issues. |
There was a problem hiding this comment.
LGTM, as Marten already reviewed anyway. A few comments, new context safety tests could be nice, but I can live without most changes here.
Overall, it is nice and simple (I do think this code can use more love, which this old comment was about, but I had expected that this might end up changing more, forcing other refactors a bit).
|
Last push responds to all the review comments from last week. Thanks all for taking a look at this. |
b340c76 to
02506cc
Compare
|
The last pushes respond to review comments and add a release note. I also figured out how to write async tests using the same patterns as the threading tests, cribbing off the errstate tests. It turns out |
There was a problem hiding this comment.
Didn't look super carefully, but only found three nitpicks now, so approving.
Not merging yet, though, since I did wonder a bit whether it would make sense to subclass ContextDict(ContextVar) and make it more dict-like - in particular, one would add __getitem__, __or__ and copy. But perhaps that would just hide too much for too little gain.
a76d841 to
fe9add9
Compare
I don't think it's worth it. The explicit |
|
Thanks, @ngoldbaum. Yeah, if we had to get individual options a lot, a class might be nice. But as is, I also prefer it as is. |
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>
This is a re-do of #26345, I'm taking over from @mtsokol because this is needed for the free-threaded work.
The new
_printoptions.pyfile 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_modefor 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.I also looked at Sebastians comment from the old PR and I'm not totally sure what he wants. It's not immediately clear to me how to expose the context variable we're creating as an internal-only context manager, if that's even what Sebastian was getting at.