Skip to content

Conversation

@peterhollender
Copy link
Contributor

Updates the codebase to be less expectant of particular dimension names, and removes the ability to specify them in SimSetup, as this level of configuration doesn't provide enough benefit to overcome the risk of confusion
Closes #364

@peterhollender peterhollender linked an issue Jul 25, 2025 that may be closed by this pull request
@peterhollender peterhollender requested a review from arhowe00 July 25, 2025 13:25
@ebrahimebrahim ebrahimebrahim self-requested a review July 28, 2025 12:41
@ebrahimebrahim ebrahimebrahim self-assigned this Jul 28, 2025
peterhollender added a commit that referenced this pull request Jul 28, 2025
Comment on lines -20 to -25
dims: Annotated[Tuple[str, str, str], OpenLIFUFieldData("Dimension keys", "Codenames of the axes in the coordinate system being used")] = ("lat", "ele", "ax")
"""Names of the axes in the coordinate system being used"""

names: Annotated[Tuple[str, str, str], OpenLIFUFieldData("Dimension names", "Human readable names of the axes in the coordinate system being used")] = ("Lateral", "Elevation", "Axial")
""""Human readable names of the axes in the coordinate system being used"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this we should we should update the example data in dvc -- I will push that change shortly

Copy link
Collaborator

@ebrahimebrahim ebrahimebrahim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you!

(force pushing to adjust a commit message)

@ebrahimebrahim
Copy link
Collaborator

(also rebasing to handle a merge conflict)

@ebrahimebrahim ebrahimebrahim force-pushed the 364-inconsistent-use-of-dimension-names branch from 15e02f4 to 50e6943 Compare July 29, 2025 12:54
@ebrahimebrahim ebrahimebrahim enabled auto-merge (rebase) July 29, 2025 12:57
@ebrahimebrahim ebrahimebrahim merged commit 010afbe into main Jul 29, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent use of dimension names

3 participants