refactor!: Consistent conversion rules and a frame-tag design for CartesianCoords

[icweaver]

This PR aims to simplify the various convert/constructor methods in this package by redesigning them around two rules:

1. Specified type parameters are honored exactly, unspecified ones are inferred from the input.

• convert(CartesianCoords, c) keeps the input's frame and element type
• convert(CartesianCoords{GalCoords}, c) rotates and infers the element type
• convert(CartesianCoords{GalCoords, Float32}, c) returns exactly that type (as the convert contract requires)

2. Everything reduces to two orthogonal primitives.

• representation change (cartesian/spherical) and frame rotation (rotmat)
• Constructors delegate to convert
• convert composes the primitives

Below is a summary of what this redesign buys us.

TC is now a frame tag

The TC parameter of CartesianCoords{TC, TF} is only ever used to pick a rotation matrix and to rebuild a spherical coordinate. Both need the frame identity (including static parameters like the FK5 equinox), not the element type of the angles that produced the vector. Storing a fully parameterized type there duplicated the element type and even allowed contradictory states like CartesianCoords{ICRSCoords{Float64}, Float32}.

TC is now canonically the element-type-free constructor (ICRSCoords, FK5Coords{2000}, etc.), with the element type carried by TF alone:

julia> c = ICRSCoords{Float32}(0.1, 0.2)
ICRSCoords{Float32}(0.1f0, 0.2f0)

julia> cartesian(c)
CartesianCoords{ICRSCoords, Float32}(Float32[0.9751704, 0.0978434, 0.19866933])

This also matches how the rest of the package already treated TC (rotmat signatures, the NearestNeighbors.jl extension, the test suite). Dispatch gets simpler too: f(c::CartesianCoords{ICRSCoords}) now matches directly, no {<:ICRSCoords} needed.

Parameterized tags remain valid: the tag's element type then determines the data's element type (e.g., CartesianCoords{ICRSCoords{Float16}}(c) holds Float16 data), and a conflicting explicit TF throws an ArgumentError, so the tag and the stored vector can never disagree.

Fixes

All of these were broken before (MethodError, ambiguity, or wrong result):

julia> c |> CartesianCoords # Was: MethodError (rotmat(CartesianCoords, ...))
CartesianCoords{ICRSCoords, Float32}(Float32[0.9751704, 0.0978434, 0.19866933])

julia> c |> CartesianCoords{ICRSCoords, Float32} # Was: constructor ambiguity
CartesianCoords{ICRSCoords, Float32}(Float32[0.9751704, 0.0978434, 0.19866933])

julia> CartesianCoords(cartesian(c)) # Was: MethodError instead of identity
CartesianCoords{ICRSCoords, Float32}(Float32[0.9751704, 0.0978434, 0.19866933])

julia> convert(CartesianCoords{GalCoords, Float32}, c) # Was: silently returned {GalCoords, Float64}, CartesianCoords{GalCoords, Float32}(...), violating the convert contract

The last one was a latent bug: convert returning a type other than the one requested breaks array setindex!, struct field assignment, and inference.

Equality and hashing

== now compares the frame tag (via ConstructionBase.constructorof) plus the values, so element types no longer need to match. For example, ICRSCoords(1.0, 2.0) == ICRSCoords{Float32}(1.0, 2.0), in the same way 1.0 == 1.0f0. Different frames (including different equinoxes) still compare unequal.

A matching Base.hash is added so value-equal coordinates of different element types collide in Dict/Set, as the hash contract requires. This turns a long-standing @test_broken into a passing test.

Breaking changes

• cartesian(c) and inferred conversions now produce the frame-tag form (CartesianCoords{ICRSCoords, Float64} instead of CartesianCoords{ICRSCoords{Float64}, Float64}). Dispatch written as CartesianCoords{<:ICRSCoords} keeps working. Only exact-parameter type checks are affected.
• == across element types of the same frame is now value-based (was always false).
• Conflicting tag/element-type combinations (CartesianCoords{ICRSCoords{Float16}, Float32}) now throw instead of silently constructing a contradictory type.
• CartesianCoords{TC}(vec::AbstractVector{<:Integer}) now floats the element type, consistent with the spherical constructors.

Follow-up

With every conversion reduced to rotmat + representation change, frames whose transform is not a rotation (e.g., FK4's E-terms in #101) can be supported by promoting the frame-change primitive to a function (frame_transform(to, from, v), defaulting to rotmat(to, from) * v). This avoids needing to define custom convert methods.

[codecov]

Codecov Report

❌ Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.51%. Comparing base (60bb231) to head (729e4c1).

Files with missing lines	Patch %	Lines
src/cartesian.jl	96.15%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #105      +/-   ##
==========================================
+ Coverage   93.63%   94.51%   +0.87%     
==========================================
  Files           8        8              
  Lines         220      237      +17     
==========================================
+ Hits          206      224      +18     
+ Misses         14       13       -1     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cgarling]

Separating the frame type from the numeric type makes sense to me
LGTM

[icweaver]

Thanks, Chris! Will wait to hear back from @aplavin or @giordano before merging

[icweaver]

Hi all, just checking back in about this. Happy to just get things moving on my end if that's preferred. Can also just check back in the next week or so if that works better for folks