Conversation
Introduce strongly-typed dataclasses for model configuration: - Dimensions, Labels, Anchoring, EstimationOptions, TransitionInfo - FactorEndogenousInfo, EndogenousFactorsInfo This improves type safety and enables IDE autocompletion while keeping user-facing model_dict as a plain dictionary. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #83 +/- ##
==========================================
- Coverage 91.02% 90.32% -0.70%
==========================================
Files 42 47 +5
Lines 3587 3919 +332
==========================================
+ Hits 3265 3540 +275
- Misses 322 379 +57 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Replace dict fields with frozendict in frozen dataclasses to ensure true immutability: - Labels.aug_periods_to_periods - Labels.aug_stages_to_stages - Anchoring.outcomes - TransitionInfo.param_names, individual_functions, function_names - EndogenousFactorsInfo.aug_periods_to_aug_period_meas_types, factor_info 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update process_model() to return a ProcessedModel frozen dataclass and update all consumers to use attribute access instead of dict access. This provides: - Better type safety with explicit typed fields - Immutability via frozen dataclass - IDE autocomplete support - Clear documentation of the model structure 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…c so that config.TEST_DATA_DIR is valid also for skillmodels the package (as opposed to the project).
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
efd2e9f to
7e7784e
Compare
| ```python | ||
| from skillmodels import ( | ||
| AnchoringSpec, | ||
| EstimationOptionsSpec, |
|
|
||
| ## Implementation Status | ||
|
|
||
| None of these correction methods are currently implemented in skillmodels. Users |
There was a problem hiding this comment.
This is probably outdated? You implemented Endogeneity correction
| Normalizations, | ||
| ) | ||
|
|
||
| MODEL2 = ModelSpec( |
There was a problem hiding this comment.
I'm quite impressed you well Claude decided which things should be dataclasses and where we keep dictionaries.
There was a problem hiding this comment.
I think this will be a thing where I'll mostly take credit for the vibes at least. Needed several iterations.
|
|
||
| ## Dimensions | ||
|
|
||
| The `Dimensions` dataclass contains integer values for model dimensions: |
There was a problem hiding this comment.
These section can now replaced with autodoc / autoclass; That's one of the benefits of strong typing! Results are best if attribute docstrings come right after the attribute like here
There was a problem hiding this comment.
Limitation of jb book for now, we can convert once available.
| from jax import Array | ||
|
|
||
|
|
||
| def _make_immutable(value: object) -> object: |
There was a problem hiding this comment.
I don't know where this is used but I tend to keep dicts and lists for data that should not be represented as a dataclass. It's more intuitive for most Python users and using tuples for lists of variables does not work well with pandas.
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class EstimationOptions: |
There was a problem hiding this comment.
Defaults should now be set in the dataclasses which gives us a much nicer way to document them. Again, another main benefit of strongly typed code. Setting defaults when everything is a dict is very annoying and it's now probably a bit scattered throughout the processing.
Refactor model configuration to use frozen dataclasses instead of dicts
Introduce strongly-typed dataclasses for model configuration:
🤖 Generated with Claude Code