-
Couldn't load subscription status.
- Fork 297
ENH: ensure yt.utils.funcs.ensure_numpy_array consistently returns a copy
#5308
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
base: main
Are you sure you want to change the base?
ENH: ensure yt.utils.funcs.ensure_numpy_array consistently returns a copy
#5308
Conversation
|
Wait, hold up. I'm not sure that we want this behavior. Making copies of every array that goes into cython is potentially going to lead to an enormous amount of memory churn and reallocation. |
yt.utils.funcs.ensure_numpy_array consistently returns a copy
|
to be clear, the only inputs where the existing code doesn't copy already are
|
|
dang, that's brutal. One of the biggest costs of ghost zones, last time I looked, was around copying and recopying arrays and checking that they were in the right units. But I suppose this probably isn't in that category. |
|
so if we don't want this to copy, that's a change in behavior in most cases, and I don't think it's one that's safe to make (people's code is going to break in subtle ways). |
|
nah it's fine, you pointed out that I was interpreting the change incorrectly. |
b307d6b to
688469b
Compare
|
Actually it was easy enough to bring only the backward-compatibility layer from the other PR, so I plan to undraft this after the on-going round of CI. |
688469b to
ade7357
Compare
ade7357 to
f0edd1a
Compare
|
even better; I can actually write this in a way that always copy and doesn't need a backward compatibility layer at all. |
PR Summary
I noticed this function could be heavily simplified, but also, and perhaps more importantly, that its return type would sometimes be a copy of the input data, but not always. This looks like a footgun so I'd rather make it always return a copy, so as to avoid subtle bugs.
PR Checklist