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

Merging Hyperspy bug-fix #126

Closed
CSSFrancis opened this issue Nov 6, 2023 · 3 comments
Closed

Merging Hyperspy bug-fix #126

CSSFrancis opened this issue Nov 6, 2023 · 3 comments

Comments

@CSSFrancis
Copy link

@jacobjma I was wondering if there is any thing blocking the merging of the commits in the abtem-dask branch. Specifically there are some bugfixes related to the hyperspy integration.

It would be good to include some of those changes into a hyperspy-bundle in the next week or so.

@jacobjma
Copy link
Member

jacobjma commented Nov 7, 2023

Can you link to exactly what you are referencing? abtem-dask is merged with main, so the branch is deprecated.

I just tested, and there was an issue with the hyperspy integration, which I fixed. See this very recent commit.

abTEM only has the most basic testing for HyperSpy integration; see here, which allows things like this to happen. I should improve this.

@CSSFrancis
Copy link
Author

Can you link to exactly what you are referencing? abtem-dask is merged with main, so the branch is deprecated.

I just tested, and there was an issue with the hyperspy integration, which I fixed. See this very recent commit.

That should fix the issue! The problem is with the underlying dask array and hyperspy axes not agreeing. I think that was fixed in 15b8a38 but that was never merged into the main repo.

abTEM only has the most basic testing for HyperSpy integration; see here, which allows things like this to happen. I should improve this.

I'll make a PR a little later this week to test the HyperSpy integration a little bit more so don't worry about it too much. I've been meaning to add some additional tutorials: https://github.com/CSSFrancis/abtem-pyxem/blob/main/AbTEMPyxem.ipynb to abTEM as well if you think something like this might be useful.

@jacobjma
Copy link
Member

This should be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants