Skip to content

DM-53979: Fix issue with attribute copying from column ref overrides#174

Merged
JeremyMcCormick merged 2 commits intomainfrom
tickets/DM-53979
Jan 29, 2026
Merged

DM-53979: Fix issue with attribute copying from column ref overrides#174
JeremyMcCormick merged 2 commits intomainfrom
tickets/DM-53979

Conversation

@JeremyMcCormick
Copy link
Collaborator

@JeremyMcCormick JeremyMcCormick commented Jan 29, 2026

Checklist

  • Ran Jenkins
  • Added a release note for user-visible changes to docs/changes

@JeremyMcCormick JeremyMcCormick changed the title Fix issue with attribute copying from column ref overrides DM-53979: Fix issue with attribute copying from column ref overrides Jan 29, 2026
@codecov
Copy link

codecov bot commented Jan 29, 2026

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.58%. Comparing base (f22fe93) to head (72dae45).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
python/felis/datamodel.py 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #174      +/-   ##
==========================================
- Coverage   91.69%   91.58%   -0.12%     
==========================================
  Files          13       13              
  Lines        2024     2031       +7     
  Branches      290      291       +1     
==========================================
+ Hits         1856     1860       +4     
- Misses        110      111       +1     
- Partials       58       60       +2     

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

@JeremyMcCormick JeremyMcCormick force-pushed the tickets/DM-53979 branch 2 times, most recently from 198d42e to ca6a6d7 Compare January 29, 2026 22:07
@JeremyMcCormick JeremyMcCormick requested a review from timj January 29, 2026 22:13
Copy link
Member

@timj timj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you know you fixed the bug if there is no test?

Copy link
Member

@timj timj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explaining that model_dump is the problem here. Please add some context to the commit message stating that the fundamental issue is that you have to avoid model_dump because it coerces types to JSON-safe form.

@JeremyMcCormick JeremyMcCormick force-pushed the tickets/DM-53979 branch 2 times, most recently from c3c119a to e6bbac1 Compare January 29, 2026 22:49
Using `model_dump` was causing conversion to JSON types here, so the
`datatype` attribute was being assigned a `str` object and not a
`StrEnum`, which broke code attempting to use the field as an enum.
Copying the attributes directly fixes this issue, as it preserves the
Python types at runtime and avoids the conversion.
@JeremyMcCormick JeremyMcCormick merged commit 5e449c1 into main Jan 29, 2026
23 of 25 checks passed
@JeremyMcCormick JeremyMcCormick deleted the tickets/DM-53979 branch January 29, 2026 22:56
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