-
Notifications
You must be signed in to change notification settings - Fork 37
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 or feature? sliced.elementCount != arr.length #313
Comments
Use
|
Maybe I didn't make it clear, what I mean is: "When user turn a 1-d array into a N-d slice, ... " in my original example, what the intended is turn a 1-d array of 6 elements into a (2, 3) 2-d slice; and if the user mistakenly write (2, 1), the library should generate at least an warning. Just as reshape does: http://mir-algorithm.libmir.org/mir_ndslice_topology.html#reshape Actually, I think function As a work-around (i.e. the intended operations):
In your modified example, the input is a 2-d array already. |
I see your point. Your issue is really that the result of
which will generate an assert error, for instance, if you use (7,1). It seems deliberate that it is a Also, you might find that
|
If you can ask the lib users to vote on this issue; or add network code to send actual usage stats to you, my gut feeling is that > 99% of the time, user want Silently perform In my program, I convert an 1-d array to 2-d slice for numerical processing; after some code change, there are more elements appended into the 1-d array, but I forget to change the sliced dimension. And the program continue to run silently without complaining anything, but the computation result is wrong: worse, silently wrong, because in numerical computations, you know, if the result is (say 1%) off the correct answer, typically user won't notice it easily. If this is in numpy, I will be alerted of the mis-match new dimension problem easily. That's why I'm suggesting: |
And for the current
The point is that user do it explicitly, and the library enforce the assertion that |
should be fixed in |
Excellent. Thanks. |
It may take a while. |
It is a breaking change, but quite small for a release. Needs to wait another one breaking change. |
I'm patient :-) |
I saw this new user's mistake: https://forum.dlang.org/post/[email protected] i.e. sliced -> fuse Since there will be a new breaking (or deprecation) change release, I'd want to suggest we should deprecate both
|
It's v3.14.14 now :-) or it's done already? |
Oh, it isn't. To be honest this change will require to much efforts to update all the codebase we support. Instead, we can add |
Expected:6
Actual:2
When user turn an array into a slice, I think most of the time they want turn the whole array into a N-d slice; if this is a feature, I'd think it's a mis-feature, should be better documented (so no surprise to the user), and maybe also issue a run-time warning by default, if the converted
sliced.elementCount != arr.length
The text was updated successfully, but these errors were encountered: