Design review report

[kdw503]

---

Design Review — RegisterDeformation

Likely Design Issues

1. GridDeformation defines == but not hash (src/griddeformation.jl:128)
   This is a correctness bug. Two GridDeformation objects that compare == will hash differently (identity-based default), violating Julia's contract that isequal(a,
   b) implies hash(a) == hash(b). Any user storing deformations in a Dict or Set will get silent misbehavior.

2. Five internal names exported with acknowledged TODO comments
   Three are flagged in the source itself (arraysize, vecindex, vecgradient!). Two more (getindex!, centeraxes) are de-facto internal. getindex! is particularly
   fragile — it calls Base._unsafe_getindex!, extending a Base private API to downstream packages.

3. The legacy affine cluster is still fully exported, labeled for removal in source
   The export block literally reads # old AffineTransfroms.jl code (TODO?: remove) followed by tformtranslate, tformrotate, tformeye, rotation2, rotation3,
   rotationparameters, transform!, transform. These pre-date CoordinateTransformations/Rotations maturation. They blur the package's identity and duplicate
   ecosystem functionality.

4. NodeIterator is the return type of exported eachnode but is not exported
   Users cannot type-annotate it, dispatch on it, or discover it from names(RegisterDeformation).

---

Design Questions

Q1. AbstractDeformation has only one concrete subtype (GridDeformation; InterpolatingDeformation is a type alias). Was a second concrete type ever planned, or is
this a load-bearing dispatch target for downstream packages?

Q2. compose(ϕ_old, ϕ_new) always returns (GridDeformation, Array{SMatrix}) — Jacobians are always computed. The gradient-free path is the functor syntax
ϕ_old(ϕ_new), which is non-obvious. Was this split intentional? Should compose accept gradient=false?

Q3. WarpedArray is exported, but constructing one directly requires the unexported to_etp for non-default extrapolation. Is WarpedArray intended for direct user
construction, or is it purely an implementation detail of warp?

Q4. GridDeformation is callable only when u is an interpolant, but this isn't enforced by the type system — calling a non-interpolating deformation fails at
runtime. Should the callable interface live only on InterpolatingDeformation, and should that alias be exported so users can annotate with it?

Q5. The deprecations in deprecated.jl (ϕ.knots, old constructors, eachknot, knotgrid, etc.) suggest an in-progress transition. Were these targeted for removal in
a future version?

Q6. translate(A, displacement) shifts an array in pixel space. CoordinateTransformations.Translation exists and is idiomatic in the ecosystem. Could the name
cause confusion, and would shift or pixel_translate be clearer?

---

Observations

• Pkg is a hard [deps] entry (not [extras]) solely for a distributed-worker activation call in warp.jl:106 — this loads Pkg for all users even when nworkers=1. A
  Julia 1.9+ package extension would make this optional.
• similarϕ uses a Unicode name in a public API — unusual for discoverability; users must know to type \phi.
• center and convert_to_fixed appear in tests directly but are unexported; if downstream packages need them, public without export would document that intent.

---

Overall

The GridDeformation core — construction, interpolation lifecycle, composition, warping — is coherent and well-designed. The main tension is between this clean
core and two layers of accumulated code: the affine transform cluster (already self-identified for removal) and the low-level helpers that leaked into the export
list. Together they inflate the export surface by roughly 40%.

Highest-leverage changes: (1) add Base.hash to GridDeformation — this is a correctness fix; (2) deprecate and unexport the legacy affine cluster to clarify the
package's identity.