Throw an exception in Python bindings on Get() buffer type mismatch #3544
Conversation
|
Good point on the test. Added test code modified from issue #3538 to use default file engine instead of streaming. |
| except Exception as e: | ||
| got_exception = 1 | ||
| logging.info(str(e)) | ||
| if not got_exception: |
There was a problem hiding this comment.
Python has a more convenient construction for that:
try:
# Some Code....
except:
# Handling of exception (if required)
else:
# execute if no exception
There was a problem hiding this comment.
Interesting... My knowledge of Python is pretty superficial, but that would eliminate a variable...
| @@ -129,6 +129,12 @@ void Engine::Get(Variable variable, pybind11::array &array, const Mode launch) | |||
| #define declare_type(T) \ | |||
| else if (type == helper::GetDataType<T>()) \ | |||
| { \ | |||
| if (!array.dtype().is(pybind11::dtype::of<T>())) \ | |||
There was a problem hiding this comment.
If the type and size of received data is known, we could just reallocate the buffer to the received type.
There was a problem hiding this comment.
Yes, we have other options. We could also redefine Get() so that it simply returned a buffer of the right type instead of asking the user to provide a buffer that might potentially be of the wrong type. (See notes in issue #3538.) The current structure is likely more the result of directly translating the C++ bindings into Python without a lot of thought of what a redesign should look like. A redesign might be desirable, but not for this release. For this, I did the minimum that would have let us know about a problem that came up in that issue. Want to create an issue for rethinking the Python APIs?
There was a problem hiding this comment.
Probably it should be improved rather on the core level, not in the python binding.
There was a problem hiding this comment.
I'm happy to hear ideas about that, but I do think that Python has specific capabilities (and flexibilities) that aren't present in C++, C, and Fortran, specifically regarding type flexibility. There will be things that we could do in Python that simply aren't easily replicated in type-safe languages (at least not efficiently) and those are the sorts of things that could be more easily addressed in bindings, and would likely be difficult to mirror all the way into core.
Relevant to issue #3538