Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v2.x #76 +/- ##
=======================================
Coverage 96.77% 96.77%
=======================================
Files 2 2
Lines 341 341
Branches 17 17
=======================================
Hits 330 330
Misses 6 6
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/LineageTree/__init__.py
Outdated
| @@ -1,4 +1,4 @@ | |||
| __version__ = "2.0.1" | |||
| __version__ = "2.1.1" | |||
There was a problem hiding this comment.
Is that really a minor change and not a patch?
There was a problem hiding this comment.
Oh indeed it should be a patch
| prop_name: prop | ||
| for prop_name, prop in lT.__dict__.items() | ||
| if isinstance(prop, dict) | ||
| if (isinstance(prop, dict) or prop_name == "_time_resolution") |
There was a problem hiding this comment.
Should we think of possible other properties?
Probably there is no strong reason for only allowing dictionary properties.
That being said, we only want lineage Tree properties and not other "weird" things/dunder properties and so on.
There was a problem hiding this comment.
This is for older files only v1.x, so I don't think we should overthink that. There is only one property we used in this version: time_resolution.
There was a problem hiding this comment.
sure, it is marginally impacting anyway ...
This reverts commit c212ad1.
leoguignard
left a comment
leoguignard
left a comment
There was a problem hiding this comment.
Can you still bump as patch before merging?
| prop_name: prop | ||
| for prop_name, prop in lT.__dict__.items() | ||
| if isinstance(prop, dict) | ||
| if (isinstance(prop, dict) or prop_name == "_time_resolution") |
There was a problem hiding this comment.
sure, it is marginally impacting anyway ...
2 participants
We only allow dict properties to be loaded. Some properties that are not dict should not be loaded, while others are important like the time resolution. Should we change the loader or add each important non dict attribute manually?