Flatten Diffraction Vector Bugfix #1024
Conversation
| elif self._is_object_dtype: | ||
| self.ragged = True | ||
| elif self.ragged == True: | ||
| self.ragged = False |
There was a problem hiding this comment.
This looks weird to me. If the first elif clause is met then the second elif clause will also be met. If that's by design it's confusing, but I think it's more likely a bug.
There was a problem hiding this comment.
Let me add some comments and see if I can't clean up the logic.
That is needed partially because upstream in hyperspy the ragged flag is a bit of a mess....
Sometimes you have data that isn't an object but hyperspy thinks that it is ragged in which case you want to say that it isn't ragged. Sometimes you have a signal with data=None in the process of initializing the signal in which case you just want to pass through without setting anything and sometimes you have an signal with a object datatype that isn't flagged as ragged....
pyxem/signals/diffraction_vectors.py
Outdated
| if self.ragged: | ||
| ragged = True | ||
| else: | ||
| ragged = False |
There was a problem hiding this comment.
Nothing actually 😅 Deleted.
@pc494 Sounds good! I might have to think about how we could change things in hyperspy to make this work a bit better. Unfortunately, handling ragged things in hyperspy is still a bit of a "beta" feature mostly because dask and numpy have different behaviors with regard to slicing object arrays. (Dask will keep the object dtype and numpy will return the underlying array). |
If you are comfortable that the code currently works I think a few comments might be enough. I hope you can understand why I found reading some of it a bit strange, now I understand that it's due to some odd upstream behaviour I'm sympathetic to the need to 'fudge' things occasionally. |
|
I think this is still with @CSSFrancis but please tag me if that's not the case. |
CSSFrancis
force-pushed
the
flatten_dv_bugfix
branch
from
0555028
to
70e7d8b
Compare
|
@pc494 this is probably good to review again |
|
Perfect should be able to get to this today :) |
name: Flatten Diffraction Vector Bugfix
about: Fixes a bug when flattening diffraction vectors
Checklist
What does this PR do? Please describe and/or link to an open issue.
There was a bug for flattening diffraction vectors when the scales/number of axes was wrong. This causes problems with 5D datasets.