-
Notifications
You must be signed in to change notification settings - Fork 21
change kwargs in read4stem #166
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
base: dev
Are you sure you want to change the base?
Conversation
CSSFrancis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some small comments.
| **kwargs: dict | ||
| Additional keyword arguments to pass to the Dataset4dstem constructor. | ||
| Additional keyword arguments to pass to the file reader. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add sampling, Orgin and Units as Other Parameters:
| Other Parameters | |
| ---------------- | |
| name : str | None, optional | |
| A descriptive name for the dataset. If None, defaults to "4D-STEM dataset" | |
| origin : NDArray | tuple | list | float | int | None, optional | |
| The origin coordinates for each dimension. If None, defaults to zeros | |
| sampling : NDArray | tuple | list | float | int | None, optional | |
| The sampling rate/spacing for each dimension. If None, defaults to ones | |
| units : list[str] | tuple | list | None, optional | |
| Units for each dimension. If None, defaults to ["pixels"] * 4 | |
| signal_units : str, optional | |
| Units for the array values, by default "arb. units" |
| sampling_override = kwargs.pop("sampling", None) | ||
| origin_override = kwargs.pop("origin", None) | ||
| units_override = kwargs.pop("units", None) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| name_override = kwargs.pop("name", None) |
| sampling = ( | ||
| sampling_override | ||
| if sampling_override is not None | ||
| else [ax["scale"] for ax in imported_axes] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all axes have offsets and scales. For example sometimes there will be axes with non-linear scales. Then you can acess the non-linear axis via ax["axis"]
| else [ax["scale"] for ax in imported_axes] | |
| else [ax.get("scale", 1) for ax in imported_axes] |
| "origin", | ||
| [ax["offset"] for ax in imported_axes], | ||
| origin = ( | ||
| origin_override if origin_override is not None else [ax["offset"] for ax in imported_axes] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above:
Not sure the policy on logging but you could output a warning that the scale is non-linear.
| origin_override if origin_override is not None else [ax["offset"] for ax in imported_axes] | |
| origin_override if origin_override is not None else [ax.get("offset", 0) for ax in imported_axes] |
| units = kwargs.pop( | ||
| "units", | ||
| ["pixels" if ax["units"] == "1" else ax["units"] for ax in imported_axes], | ||
| units = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All axes will have units :) and a name. That is the minimal definition.
| array=imported_data["data"], | ||
| sampling=sampling, | ||
| origin=origin, | ||
| units=units, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| units=units, | |
| units=units, | |
| name = name, |
No description provided.