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

✨ feat(quantity): enable non-Array quax.ArrayValue as Quantity's value #358

Merged
merged 2 commits into from
Jan 29, 2025

Conversation

nstarman
Copy link
Contributor

@nstarman nstarman commented Jan 7, 2025

Part of the push for unxt v1.1, to enable more value types, e.g. the low rank array in the test.

@nstarman nstarman added this to the v1.1.0 milestone Jan 7, 2025
@nstarman nstarman requested review from adrn and jnibauer January 7, 2025 15:05
@nstarman nstarman marked this pull request as ready for review January 7, 2025 15:07
Copy link

codecov bot commented Jan 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.75%. Comparing base (a7887bb) to head (974e657).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #358      +/-   ##
==========================================
+ Coverage   97.72%   97.75%   +0.03%     
==========================================
  Files          45       46       +1     
  Lines        1579     1606      +27     
  Branches      316      320       +4     
==========================================
+ Hits         1543     1570      +27     
  Misses         29       29              
  Partials        7        7              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@adrn adrn left a comment

Choose a reason for hiding this comment

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

Seems like a good idea! On the name: maybe convert_to_value or convert_to_unxt_value instead of noun _converter?

@nstarman nstarman force-pushed the quantity/fix-value-converter branch from 72e9b00 to dc63a81 Compare January 22, 2025 17:57
@nstarman
Copy link
Contributor Author

@adrn Agreed. I pushed another gross name. It would be good to think of a good one 😆.

@nstarman nstarman force-pushed the quantity/fix-value-converter branch from dc63a81 to 0fab4b0 Compare January 22, 2025 21:12
@adrn
Copy link
Contributor

adrn commented Jan 29, 2025

How about just convert_to_quantity_value?

Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
@nstarman nstarman force-pushed the quantity/fix-value-converter branch from 0fab4b0 to 974e657 Compare January 29, 2025 22:55
@nstarman
Copy link
Contributor Author

Changed! Let's get it in.

@nstarman nstarman merged commit d90350f into GalacticDynamics:main Jan 29, 2025
22 checks passed
@nstarman nstarman deleted the quantity/fix-value-converter branch January 29, 2025 23:46
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