strip_module_name: handle UnionAll component types#583
Conversation
PSY6 parameterized some component families on a unit system (e.g.
ReserveDemandCurve{ReserveUp} is now ReserveDemandCurve{ReserveUp, U} where U),
making the partially-applied type a UnionAll. The @generated Type method read
T.parameters, which only exists on DataType, so it threw a FieldError on these
types. Unwrap the UnionAll and drop the free TypeVars, keeping the bound
parameters so the stripped name stays unique (e.g. "ReserveDemandCurve{ReserveUp}").
This lets downstream consumers (IOM container-key encoding) drop their local
UnionAll workaround.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request updates strip_module_name(::Type) to correctly handle partially-applied parametric component types that are represented as UnionAll (e.g., when a component family adds a free type parameter like a unit system), preventing FieldError from accessing .parameters on non-DataType types. This supports downstream consumers that rely on stable, unique, module-stripped type strings (e.g., container-key encoding).
Changes:
- Unwrap
UnionAlltypes before inspecting parameters and drop freeTypeVarparameters while keeping bound parameters for uniqueness. - Improve parameter stringification to handle non-
Typeparameters (e.g., value parameters) without error.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| params = filter(p -> !(p isa TypeVar), collect(base.parameters)) | ||
| if isempty(params) | ||
| return name | ||
| else # I believe there's only 2 parameters, so slightly overkill. |
| base = T isa UnionAll ? Base.unwrap_unionall(T) : T | ||
| name = string(nameof(base)) | ||
| params = filter(p -> !(p isa TypeVar), collect(base.parameters)) | ||
| if isempty(params) |
There was a problem hiding this comment.
We could add a test but we couldn't test if it actually removed a module name without adding a whole submodule.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## IS4 #583 +/- ##
=======================================
Coverage 83.50% 83.51%
=======================================
Files 74 74
Lines 6264 6266 +2
=======================================
+ Hits 5231 5233 +2
Misses 1033 1033
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
PSY6 parameterized some component families on a unit system (e.g. ReserveDemandCurve{ReserveUp} is now ReserveDemandCurve{ReserveUp, U} where U), making the partially-applied type a UnionAll. The @generated Type method read T.parameters, which only exists on DataType, so it threw a FieldError on these types. Unwrap the UnionAll and drop the free TypeVars, keeping the bound parameters so the stripped name stays unique (e.g. "ReserveDemandCurve{ReserveUp}").
This lets downstream consumers (IOM container-key encoding) drop their local UnionAll workaround.