-
Notifications
You must be signed in to change notification settings - Fork 225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
misc: Fix EnrichedTuple rebuilding #2418
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2418 +/- ##
=======================================
Coverage 86.99% 86.99%
=======================================
Files 236 236
Lines 44821 44837 +16
Branches 8360 8361 +1
=======================================
+ Hits 38991 39007 +16
Misses 5101 5101
Partials 729 729 ☔ View full report in Codecov by Sentry. |
could you also rebase the commit history as a single commit? let me know if you need help with rebasing |
@FabioLuporini I mean, is it? Arguably what's more of a hack is overriding instance I saw that I think ideally the hierarchy would be refactored such that those instance hacks aren't used anywhere and the attributes are explicit; that's why I at least thought the change should be made for |
I think it's perfectly legal to alter That being said, I think I agree with the rest you've said. We should maybe just wrap a dict and pull from it each time we enter |
Fair enough. So for now should I leave the explicit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for me this is now GTG, thanks. @mloubout ?
GTG, need rebase |
Made some changes requested on Slack; refactored Also added |
misc: refactor EnrichedTuple, fix DimensionTuple rebuilding misc: clean up EnrichedTuple/DimensionTuple misc: remove DimensionTuple rkwargs misc: EnrichedTuple._getters -> getters test: add EnrichedTuple._rebuild test
Fixes
EnrichedTuple
(and its hierarchy) rebuilding to an empty instance.