-
Notifications
You must be signed in to change notification settings - Fork 55
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
Use readonly morphio Morphology #979
Conversation
eleftherioszisis
commented
Feb 9, 2022
•
edited
Loading
edited
- Use exclusively morphio's readonly Morphology
- Morphology inheritance turned into composition
- Direct mutation removed
- Section booleaness removed as it is more confusing than useful
It looks like there is no equality defined for readonly sections: def test_iter_sections_default():
ref = [s for n in POP.neurites for s in n.iter_sections()]
> assert (ref ==
[n for n in iter_sections(POP)])
E assert [Section(id=0...dren: 0>, ...] == [Section(id=0...dren: 0>, ...]
E At index 0 diff: Section(id=0, type=2, n_points=11)<parent: Section(id=None), nchildren: 2> != Section(id=0, type=2, n_points=11)<parent: Section(id=None), nchildren: 2>
E Use -v to get the full diff
tests/core/test_iter.py:103: AssertionError |
34cc5aa
to
f1396b5
Compare
Codecov Report
@@ Coverage Diff @@
## v4 #979 +/- ##
======================================
Coverage ? 100.00%
======================================
Files ? 36
Lines ? 2482
Branches ? 0
======================================
Hits ? 2482
Misses ? 0
Partials ? 0 |
it's fine most of our code have neurom<4 I think :P |
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.
LGTM, thanks!
def __nonzero__(self): | ||
"""If has root node.""" | ||
return bool(self.morphio_root_node) | ||
|
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.
@mgeplf you mentioned you don't agree with removing the section booleaness. Is this still true?
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.
no; I can see how it's confusing; one can check if there are children more explicitly.
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.
LGTM
Switch to using the morphio readonly morphology instead of mut Co-authored-by: Mike Gevaert <[email protected]>
* Mixed subtree processing (#981) * Refactor tests for test_mixed.py (#1027) * Remove deprecated modules and functions/classes & warnings (#1026, #1032) * Use readonly morphio Morphology by default (#979) * Morphology level radial distance features use the soma as reference point (#1030) * Expose subtree processing from the morph_stats api (#1034) * Remove pyXX prefix for lint, docs, and coverage (#1038) * Fix tutorials and add tutorial testenv (#1039) * Add isort for formatting/linting (#1040) * Add testing of example scripts (#1041) * Make documentation/docstrings testable (#1035) * Add black to neurom, format everything, and add to lint (#1042) * Fix load_morpholgies to always resolve paths (#1047) * allow Morphology objects to be either mut or immut (#1049) * Replace iter_* methods by properties in core objects and improve iter_segments (#1054) * Decouple Morphology constructor from io (#1120) * Move soma methods to functions (#1118)