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

Fix behavior for nested types #1

Merged
merged 7 commits into from
Sep 25, 2023
Merged

Fix behavior for nested types #1

merged 7 commits into from
Sep 25, 2023

Conversation

MilesCranmer
Copy link
Member

Now when you have a Quantity{Measurement{Float32}}, it will return Float32, rather than Measurement{Float32}.

The change is essentially just

 function _base_numeric_type(::Type{T}) where {T}
     params = T isa UnionAll ? T.body.parameters : T.parameters
     if isempty(params)
         return T
     else
+        return _base_numeric_type(first(params))
-        return first(params)
     end
 end

@stevengj could you please confirm this is your expected behavior? Note that this will result in base_numeric_type(Quantity{ComplexF32}) returning Float32, which I believe is more correct, as Float32 is the base type of ComplexF32.

@stevengj
Copy link

I don’t think you should try to guess from the order of the parameter listing. Packages implementing new numeric types should overload this function themselves.

@MilesCranmer
Copy link
Member Author

I don’t think you should try to guess from the order of the parameter listing. Packages implementing new numeric types should overload this function themselves.

I agree. The default behavior is very simple and just assumes the first type is the numeric base type. It’s more of a useful fallback that works 99% of the time, not a final solution.

But I am emphasizing that a downstream package should write a specialized method to their type.

@MilesCranmer
Copy link
Member Author

Here's the updated readme which is a bit more explicit about this.

This package currently exports a tiny function base_numeric_type that
extracts the base numeric type from a numeric type T:

  • base_numeric_type(::Type{T}) where {T}
  • base_numeric_type(x::T)

For example,

Input Type Output Type
Float32 Float32
ComplexF32 Float32
Measurement{Float32} Float32
Dual{BigFloat} BigFloat
Dual{ComplexF32} Float32
Rational{Int8} Int8
Quantity{Float32, ...} Float32
Quantity{Measurement{Float32}, ...} Float32

Package maintainers should write a specialized method for their type.
For example, to define the base numeric type for a dual number, one could write:

import BaseType: base_numeric_type

base_numeric_type(::Type{Dual{T}}) where {T} = base_numeric_type(T)

It is important to call base_numeric_type recursively like this to deal with
nested numeric types such as Quantity{Measurement{T}}.

@MilesCranmer
Copy link
Member Author

Once the new version is updated we can ask different packages to start overloading this.

@MilesCranmer MilesCranmer merged commit 4951963 into main Sep 25, 2023
2 checks passed
@MilesCranmer MilesCranmer deleted the nested-types branch September 25, 2023 10:14
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