-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: track gain and mdoc files alongside runs #280
Conversation
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.
What happened to per_section_parameters ? When I look at the changes, they are removed in the new changes.
We're going to remove them for now because they'll be coming in a later update. It wouldn't hurt to just include them early, except that I'm worried we'll be making breaking changes (like removing/renaming fields in these tables) once we start working with that data, so in that case it would be better to add them in once we're more confident that the structure of those tables is stable. |
maximum_value: 180 | ||
unit: | ||
symbol: ° | ||
descriptive_name: degrees | ||
PerSectionAlignmentParameters: | ||
name: PerSectionAlignmentParameters |
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.
- The
in_plane_rotation
field will be a collection of 4 float values. Should we make it a vector or a string? - The
beam_tilt
field is not going to be supported for now. - We should add support for the
is_canonical
boolean field and the floatvolume_x_rotation
.
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.
Supporting vectors and other strongly typed json fields is pretty doable in the API - we already support 2d arays of floats, so supporting 1d arrays is easy to add. I haven't tried to add complex types to the client before though, so that could get interesting. Do you think in_plane_rotation
would be best represented as a list of floats?
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.
@uermel Could we have your thoughts on what is the best way to represent in_plane_rotation
for the end user?
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.
Decision: This will be a 2d rotation matrix, basically list[list[float]]
55f281c
to
368878a
Compare
This is a draft to make sure we're aligned on the structure (and naming!) for keeping track of mdoc and gains files associated with a run. Once we agree on names/structures, I'll update the API with the changes.
Also feel free to just add commits to this branch if that's easier.