Add dimensionality information to Specs#89
Conversation
DiamondJoseph
commented
Aug 30, 2023
DiamondJoseph
commented
- Closes Specs should be able to get their dimensionality information #88
- Add axes, shape and snaked information in (usually) non-points-calculating method.
- Deprecate Spec.axes: did not respect Frame information
- Deprecate Spec.shape: Called calculate() and discarded information
- Closes #88 - Add axes, shape and snaked information in (usually) non-points-calculating method. - Deprecate Spec.axes: did not respect Frame information - Deprecate Spec.shape: Called calculate() and discarded information
| class DimensionInfo(Generic[Axis]): | ||
| def __init__( | ||
| self, | ||
| axes: Tuple[Tuple[Axis, ...]], | ||
| shape: Tuple[int, ...], | ||
| snaked: Tuple[bool, ...] = None, | ||
| ): | ||
| self._axes = axes | ||
| self._shape = shape | ||
| self._snaked = snaked or (False,) * len(shape) | ||
|
|
||
| @property | ||
| def axes(self) -> Tuple[Tuple[Axis, ...]]: | ||
| return self._axes | ||
|
|
||
| @property | ||
| def shape(self) -> Tuple[int, ...]: | ||
| return self._shape | ||
|
|
||
| @property | ||
| def snaked(self) -> Tuple[bool, ...]: | ||
| return self._snaked | ||
|
|
||
|
|
There was a problem hiding this comment.
Achieves the same thing and more inkeeping with the rest of the codebase
| class DimensionInfo(Generic[Axis]): | |
| def __init__( | |
| self, | |
| axes: Tuple[Tuple[Axis, ...]], | |
| shape: Tuple[int, ...], | |
| snaked: Tuple[bool, ...] = None, | |
| ): | |
| self._axes = axes | |
| self._shape = shape | |
| self._snaked = snaked or (False,) * len(shape) | |
| @property | |
| def axes(self) -> Tuple[Tuple[Axis, ...]]: | |
| return self._axes | |
| @property | |
| def shape(self) -> Tuple[int, ...]: | |
| return self._shape | |
| @property | |
| def snaked(self) -> Tuple[bool, ...]: | |
| return self._snaked | |
| @dataclass(frozen=True) | |
| class DimensionInfo: | |
| axes: Tuple[Tuple[Axis, ...]] | |
| shape: Tuple[int, ...] | |
| snaked: Tuple[bool, ...] |
There was a problem hiding this comment.
Also would prefer Dimensions to DimensionInfo, because that's also a similar naming convention to other classes
There was a problem hiding this comment.
It's not in keeping with the other classes in this file, which is why I didn't make a dataclass. it may be a holdover from worse dataclass Generic handling? I'll see how it behaves and maybe make a PR to move Frame etc. over to be dataclasses: the specs are dataclasses but the Frames etc. are not.
RE: Dimensions vs DimensionInfo: Dimension has meaning from SPG that is already present in some of the docs. This object is not a Dimension or collection of Dimensions, it describes some Dimensions.
Each component of axes/shape/snaked refers to the behaviour of a single Dimension. DimensionsInfo for maybe
There was a problem hiding this comment.
Could also suggest Dimensionality
There was a problem hiding this comment.
Dimensionality I like
There was a problem hiding this comment.
Good point, none of the other classes in core are dataclasses, but they do all contain a lot of logic, so I think that's correct. This almost purely holds data.
From discussion with Tom, he's leaning towards suggesting that there be no default for snaked and that be dealt with downstream. That would then make this a pure data class.
| """ | ||
| raise NotImplementedError(self) | ||
| warn( | ||
| "axes() is deprecated, call dimension_info()", |
There was a problem hiding this comment.
Not sure this should be deprecated. I think there are still valid reasons to just say "I want a list of all axes that may be involved in this scan" without caring about/having to unpack the dimensions.
There was a problem hiding this comment.
AIUI Tom requested deprecating axes(), shape(), will have to check when he's back.
The handling to get from the info is there, so it's easy to leave them both in and not worry about it again.
There was a problem hiding this comment.
Chatted with Tom, happy to deprecate for now and and add a .flat_axes() to Dimensionality if a need arises
| ): | ||
| self._axes = axes | ||
| self._shape = shape | ||
| self._snaked = snaked or (False,) * len(shape) |
There was a problem hiding this comment.
Unsure if this would help, but is it easier to make snaked an enum of yes, no and unknown?
There was a problem hiding this comment.
Snaked is False until an inner Dimension when it's True. Maybe it should be the outermost Dimension that is Snaked instead, or a dimension index?
| self, | ||
| axes: Tuple[Tuple[Axis, ...]], | ||
| shape: Tuple[int, ...], | ||
| snaked: Tuple[bool, ...] = None, |
There was a problem hiding this comment.
| snaked: Tuple[bool, ...] = None, | |
| snaked: Optional[Tuple[bool, ...]] = None, |
|
What's the status of this now? |
3 participants
