-
Notifications
You must be signed in to change notification settings - Fork 603
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
api: first draft for removing nativespec from ImageBuf #4482
base: main
Are you sure you want to change the base?
api: first draft for removing nativespec from ImageBuf #4482
Conversation
Signed-off-by: Basile Fraboni <[email protected]>
Signed-off-by: Basile Fraboni <[email protected]>
Signed-off-by: Basile Fraboni <[email protected]>
Signed-off-by: Basile Fraboni <[email protected]>
const ImageSpec& nativespec() const; | ||
TypeDesc file_format() const; | ||
/// Same as the above for channel specific formats. | ||
std::vector<TypeDesc> file_channelformats() const; |
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.
Not sure how much it matters, but do you think returning a vector is good here? As opposed to returning a const vector&
or a cspan<TypeDesc>
? (But those are only options it's stored somewhere in the IB and we can make a stable reference to it.
@@ -361,7 +361,9 @@ declare_imagebuf(py::module& m) | |||
"height"_a = 0, "depth"_a = 0) | |||
.def("spec", &ImageBuf::spec, | |||
py::return_value_policy::reference_internal) | |||
.def("nativespec", &ImageBuf::nativespec, | |||
.def("file_format", &ImageBuf::file_format, |
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.
I think here, like with C++, for back compatibility we'll want to preserve a nativespec() call, even if it does something different than before.
I think this is going in the right direction. Looks like failed tests? And you need a DCO? |
Just a remind that there are some pending items to finish this |
Second half of API changes to remove
nativespec
fromImageCache
andImageBuf
for issue: #4436nativespec()
from the ImageBuf API but this creates a bunch of problems I have not answers for inImageBuf::read