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

assign_subclass and signal with one value? #2766

Closed
magnunor opened this issue Jun 8, 2021 · 4 comments
Closed

assign_subclass and signal with one value? #2766

magnunor opened this issue Jun 8, 2021 · 4 comments

Comments

@magnunor
Copy link
Contributor

magnunor commented Jun 8, 2021

In #2703, one of the tests is failing due to how s._assign_subclass works.

Essentially, should a signal with one value be a BaseSignal, or a Signal1D? Currently, _assign_subclass turns a signal with one value into a Signal1D, but the test expects it to be a BaseSignal.

To replicate this:

s = hs.signals.BaseSignal(1) # <BaseSignal, title: , dimensions: (|1)>
s._assign_subclass() # <Signal1D, title: , dimensions: (|1)>

The question is then which signal type should a signal with one value be? BaseSignal or Signal1D?

@jlaehne
Copy link
Contributor

jlaehne commented Jun 10, 2021

I would have thought that BaseSignal is just a parent class for all other signal classes, but is normally not used. Signal1D provides many additional routines, but then most of them don't make sense on a signal with one value only. So it seems to me to indeed be an edge case.

@thomasaarholt
Copy link
Contributor

If the array is a zero-dimensional array, then I'd say it should probably go to basesignal or a "Signal0D".

@francisco-dlp
Copy link
Member

HyperSpy does not support array scalars (i.e. 0-dimensional arrays) because scalar arrays, unlike ndarrays, are immutable. That's why HyperSpy converts any array scalar in a 1D array of shape [1].

This is indeed a corner case. Considering that, as pointed out by @jlaehne, most Signal1D methods make no sense for 1-element arrays, I think that it'll be better to assign them to BaseSignal.

@ericpre
Copy link
Member

ericpre commented Sep 5, 2021

Done in #2773.

@ericpre ericpre closed this as completed Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants