-
Notifications
You must be signed in to change notification settings - Fork 299
BUG: avoid soon-to-be-deprecated direct mutations of ndarray.shape attributes (1/2) #5307
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
BUG: avoid soon-to-be-deprecated direct mutations of ndarray.shape attributes (1/2) #5307
Conversation
35d1bbf to
569c545
Compare
|
Your work on making this backward- and forward-compatible is admirable, and I wish it didn't have to be so. |
|
yeah it's a bit fiddly. Hopefully it should be straightforward to find-and-replace when the time comes. |
|
What I was trying to say without saying it is that I wish we didn't have to make this change, and I agree it looks a bit fiddly to use the |
|
Oh yeah, this has been on my radar for a while but I was constantly putting it off, but it seems to be moving forward upstream... |
|
Clearly I messed up somewhere. Help is welcome, but in the meantime, I'll be off duty for today. |
b867112 to
cbc0582
Compare
6a1a1fe to
f1f1a92
Compare
|
ok this is now reasonably stable, so, opening for review. |
2170370 to
f1f1a92
Compare
|
... I couldn't resist. Volume Rendering is now handled in its own PR to hopefully make reviewing swifter. See #5309 |
|
I have no idea how/if the C side of things will change, so I haven't updated it yet. |
chrishavlin
left a comment
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.
changes LGTM. I'm a little worried the changes in the frontends might not get picked up by tests at the moment... but all the changes look functionally identical on a careful read. So unless anyone has bandwidth to run some nosetests locally, I say we merge
…mutations of ndarray.shape attributes (1/2)
…7-on-yt-4.4.x Backport PR #5307 on branch yt-4.4.x (BUG: avoid soon-to-be-deprecated direct mutations of ndarray.shape attributes (1/2))
PR Summary
xref: numpy/numpy#29536
This is incomplete at the time of opening, I just want to take what I already have for a spin on CI
PR Checklist