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 BaseType.base_numeric_type instead of Base.one to get base scalar type #92

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MilesCranmer
Copy link

x-ref #88, JuliaPhysics/Measurements.jl#155, SymbolicML/DynamicQuantities.jl#40

I've confirmed that with this change as well as the intended change to make AbstractQuantity <: Number, DynamicQuantities.jl is now compatible with QuadGK.jl. And this allows me to keep Base.one(::Quantity) -> Quantity.

BaseType.jl should already work for all types I can think of due to the default behavior, but feel free to wait until more interfaces are explicitly declared (e.g., JuliaPhysics/Measurements.jl#155)

@MilesCranmer MilesCranmer changed the title Use BaseType.jl instead of Base.one to get scalar Use BaseType.base_numeric_type instead of Base.one to get base scalar type Sep 25, 2023
@giordano
Copy link
Member

And this allows me to keep Base.one(::Quantity) -> Quantity.

Isn't that violating the semantic of Base.one?

@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (10bc1a1) 98.16% compared to head (bdd31ec) 98.16%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #92   +/-   ##
=======================================
  Coverage   98.16%   98.16%           
=======================================
  Files           6        6           
  Lines         599      599           
=======================================
  Hits          588      588           
  Misses         11       11           
Files Changed Coverage Δ
src/QuadGK.jl 100.00% <ø> (ø)
src/gausskronrod.jl 98.37% <ø> (ø)

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

@MilesCranmer
Copy link
Author

Unitful.Quantity yes, but DynamicQuantities.Quantity no.

See the discussion in SymbolicML/DynamicQuantities.jl#40 for details.

@MilesCranmer
Copy link
Author

@stevengj wondering if you could look at merging this? It is completely compatible with existing types and also adds support for DualNumbers and DynamicQuantities (for which one is used for something different)

@MilesCranmer
Copy link
Author

FYI @mikeingold once this PR merges, you will be able to use DynamicQuantities for QuadGK.

@MilesCranmer
Copy link
Author

FYI @stevengj this is now in Measurements.jl as an extension, and also compatible with DualNumbers, Unitful, and DynamicQuantities (as well as a nested combination of any of those)

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