Repository navigation
MAINT: Remove any promotion-state switching logic - #27156
Conversation
This is the first level, no following code simplifications, just straight up deletions of any branching. I kept a UserWarning, in case someone had the bad idea to permanently set the environment variable and think they can rely on it. Although would be happy to just delete that as well.
I think we can just do that, but maybe it's prudent to just point it out anyway...
ngoldbaum
left a comment
There was a problem hiding this comment.
Always love deleting almost a thousand lines of sort of complicated C code. I left a few minor comments but this looks good.
mattip
left a comment
There was a problem hiding this comment.
LGTM, and the documentation changes make sense. I will leave this for a bit if anyone else wants to take a look.
mhvk
left a comment
There was a problem hiding this comment.
Looks good, but there are quite a few comments by @ngoldbaum that are unresolved (though most/all are addressed?), so just approving, not merging.
Also one tiny nitpick - not worth running CI on its own, but can be done if there are other changes to be made still.
| if weak_promotion and expected_weak is not None: | ||
| expected = expected_weak | ||
|
|
||
| other, expected): |
|
Sorry for not giving this another pass! Looks good to me now. @seberg feel free to merge along with Marten's suggested fixup. |
| # TODO: Remove the environment variable entirely now that it is "weak" | ||
| _core._set_promotion_state( | ||
| os.environ.get("NPY_PROMOTION_STATE", "weak")) | ||
| if (os.environ.get("NPY_PROMOTION_STATE", "weak") != "weak"): |
There was a problem hiding this comment.
Assign "weak" to a variable like DEFAULT_PROMOTION_STATE for better maintainability and readability.
This is the first level, no following code simplifications, just straight up deletions of any branching. I kept a UserWarning, in case someone had the bad idea to permanently set the environment variable and think they can rely on it. Although would be happy to just delete that as well. * DOC: Add release note for promotion state removal I think we can just do that, but maybe it's prudent to just point it out anyway... * DOC: Mention semi-private removed functions and tweak docs * Adress Marten's last comment
|
Hmmm, this removes the ability to use the env variable, so not sure about back-porting. |
|
@seberrg OK, I'll skip a backport for now. |
This is the first level, no following code simplifications, just straight up deletions of any branching. I kept a UserWarning, in case someone had the bad idea to permanently set the environment variable and think they can rely on it. Although would be happy to just delete that as well. * DOC: Add release note for promotion state removal I think we can just do that, but maybe it's prudent to just point it out anyway... * DOC: Mention semi-private removed functions and tweak docs * Adress Marten's last comment
This is the first level, no following code simplifications, just straight up deletions of any branching.
I kept a UserWarning, in case someone had the bad idea to permanently set the environment variable and think they can rely on it. Although would be happy to just delete that as well.
I don't really think this interferes with regularly touched code, so think we might as well do it now and not worry about backports (the diff isn't huge anyway)