-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
BUG: Raise ValueError on strings with smaller than expected byte-size #18427
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
Conversation
@pearu thoughts? |
In general, when the Fortran function expects a string with a fixed number of bytes and users input contains fewer bytes than expected, then for a proper program behavior we have the following options:
Here, the chosen behavior depends on the input intent:
This PR implements a solution to |
Sorry for taking so long - there are some comments that I can remove later, but I wanted to check for behavior first. Let me know what you think @pearu - thanks! |
@melissawm Do you want to keep this open? The linter is complaining about some long lines in the test. |
@charris I'll fix that and I think this is close to done. Thanks! |
Wait for my related PR re string_from_obj. I think there are cases where the input can have fewer bytes than expected, or at least, we should discuss this. |
This PR is blocked by #18759 that needs to be merged first as it defines consistent handling of string arguments. |
Now that #18759 has been merged I'll pick up work on this. |
@pearu - now that #18759 has been merged, consider the following example: subroutine string_size_inout(a)
implicit none
character(len=4), intent(inout) :: a
a(4:4) = 'A'
end subroutine string_size_inout Wrapping it with f2py and calling it from Python with |
@melissawm , first, let me explain the overall meaning of
I hope the above makes sense. But in short, yes, the current master behavior you experienced is expected: only the first 3 bytes are copied back to the input argument In retrospect, catching the silent loss of information from scalar casting or string truncation would be a nice feature. However, note that the loss of information cannot be always detected before calling the Fortran function, but one could only detect the possibility of a loss of information. The actual loss of information may or may not realize. Perhaps throwing a warning would be a more appropriate action rather than an exception? In any case, if one would be purchasing for this feature, it should be implemented for both scalars and strings at the same time and that would involve checking the changes in intermediate buffers to see if any change is out of range of input dtype and then throw an exception. |
The original issue this was meant to address has been closed as fixed, but the tests here are still good to have and nail down what we expect from string handling.. |
Let me close this and follow with with a pr with just the tests then. Thanks! |
This is an attempt at fixing #15311.
This raises a ValueError if the input array has byte-size smaller than expected, but the string is truncated if it is larger than expected (which is consistent with the current docs).
Feedback is appreciated!
Closes #15311