Skip to content
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

AD_HDF5_SINGLE: return sliceable object #32

Merged
merged 1 commit into from Sep 30, 2022

Conversation

prjemian
Copy link
Contributor

@prjemian prjemian added the bug Something isn't working label Sep 30, 2022
@prjemian prjemian self-assigned this Sep 30, 2022
@prjemian
Copy link
Contributor Author

The CI failure is because the conf.py file is out of date with current Sphinx. (need to comment the language= line now, the default is acceptable).

@danielballan
Copy link
Member

I am slightly worried this API change could break other uses of AD_HDF5_SINGLE. Would it be better to make a new handler (and new spec name) with the new behavior? Or is this actually a bug, @tacaswell?

@tacaswell
Copy link
Contributor

I think this is actually a bug and I think this change is mostly backwards compatible (what would you be doing to a list that a N-D stack of numpy arrays would not handle?).

I am in favor of merging this but making sure the SXSS AST is very aware of this.

@tacaswell
Copy link
Contributor

This may also fix a latent bug that we have not hit at SIX.

@danielballan danielballan merged commit c3db30d into master Sep 30, 2022
@danielballan danielballan deleted the 31-AD_HDF5_SINGLE-return-sliceable branch September 30, 2022 20:51
@prjemian
Copy link
Contributor Author

@prjemian
Copy link
Contributor Author

@tacaswell - Thanks for the review, and the fix code (that's yours). @danielballan - Thanks for the merge.

@danielballan
Copy link
Member

Released https://pypi.org/project/area-detector-handlers/0.0.10/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants