JuliaMath / QuadGK.jl

adaptive 1d numerical Gauss–Kronrod integration in Julia
MIT License
268 stars 37 forks source link

Use BaseType.base_numeric_type instead of Base.one to get base scalar type #92

Open MilesCranmer opened 11 months ago

MilesCranmer commented 11 months ago

x-ref https://github.com/JuliaMath/QuadGK.jl/issues/88, https://github.com/JuliaPhysics/Measurements.jl/pull/155, https://github.com/SymbolicML/DynamicQuantities.jl/issues/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., https://github.com/JuliaPhysics/Measurements.jl/pull/155)

giordano commented 11 months ago

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

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

codecov[bot] commented 11 months ago

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 ```diff @@ Coverage Diff @@ ## master #92 +/- ## ======================================= Coverage 98.16% 98.16% ======================================= Files 6 6 Lines 599 599 ======================================= Hits 588 588 Misses 11 11 ``` | [Files Changed](https://app.codecov.io/gh/JuliaMath/QuadGK.jl/pull/92?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath) | Coverage Δ | | |---|---|---| | [src/QuadGK.jl](https://app.codecov.io/gh/JuliaMath/QuadGK.jl/pull/92?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath#diff-c3JjL1F1YWRHSy5qbA==) | `100.00% <ø> (ø)` | | | [src/gausskronrod.jl](https://app.codecov.io/gh/JuliaMath/QuadGK.jl/pull/92?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath#diff-c3JjL2dhdXNza3JvbnJvZC5qbA==) | `98.37% <ø> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

MilesCranmer commented 11 months ago

Unitful.Quantity yes, but DynamicQuantities.Quantity no.

See the discussion in https://github.com/SymbolicML/DynamicQuantities.jl/issues/40 for details.

MilesCranmer commented 11 months ago

@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 commented 10 months ago

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

MilesCranmer commented 10 months ago

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)