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

Use newly-supported KVector Unitful extension to simplify differential #114

Closed
wants to merge 6 commits into from

Conversation

mikeingold
Copy link
Collaborator

@mikeingold mikeingold commented Oct 21, 2024

Status

Need to rebase after completion of #122.

Changes

  • Increases minimum compat version of CliffordNumbers.jl dependency to v0.1.9
  • Uses CliffordNumbers.jl's new Unitful extension, converting Meshes.Vec to Unitful.Quantity{KVector} which now support unit-aware wedge products
  • Add a new utility method _units(::Meshes.Vec)

end

# Extract the length units used by the CRS of a Geometry
_units(::Meshes.Geometry{M, CRS}) where {M, CRS} = Unitful.unit(CoordRefSystems.lentype(CRS))
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
_units(::Meshes.Geometry{M, CRS}) where {M, CRS} = Unitful.unit(CoordRefSystems.lentype(CRS))
function _units(::Meshes.Geometry{M, CRS}) where {M, CRS}
Unitful.unit(CoordRefSystems.lentype(CRS))
end

@brainandforce
Copy link

Just realized that you're depending on me for this PR, so I'll push 0.1.9 out for your convenience.

@mikeingold
Copy link
Collaborator Author

Just realized that you're depending on me for this PR, so I'll push 0.1.9 out for your convenience.

Thanks! This one is in line behind a much heftier PR, so no worries on the release timing.

To your credit, CliffordNumbers.jl turns one of our core functions into a borderline one-liner, which is much more elegant than the previous solution. 😄

@mikeingold
Copy link
Collaborator Author

mikeingold commented Nov 8, 2024

I had trouble getting git merge to run successfully after the huge #122 merge, so I'm closing this in favor of a new branch PR in #130.

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.

2 participants