Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Expose subtree processing from the morph_stats api #1034

Merged
merged 4 commits into from
May 12, 2022
Merged

Expose subtree processing from the morph_stats api #1034

merged 4 commits into from
May 12, 2022

Conversation

eleftherioszisis
Copy link
Contributor

Allow to process heterogeneous subtrees for morph_stats analysis.

Examples usage:

neurom.apps.morph_stats.extract_dataframe(moprh, config, use_subtrees=True)
neurom.apps.morph_stats.extract_stats(moprh, config, use_subtrees=True)

#975

@codecov-commenter
Copy link

codecov-commenter commented May 9, 2022 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (v4@006cafc). Click here to learn what that means.
The diff coverage is n/a.

@@          Coverage Diff           @@
##             v4     #1034   +/-   ##
======================================
  Coverage      ?   100.00%           
======================================
  Files         ?        35           
  Lines         ?      2464           
  Branches      ?         0           
======================================
  Hits          ?      2464           
  Misses        ?         0           
  Partials      ?         0

@adrien-berchet
Copy link
Member

LGTM but why is the default set to False? I think the results from the old behavior are a bit hard to understand for a user who probably expects the results given by use_subtrees=True by default, WDT?

@eleftherioszisis
Copy link
Contributor Author

eleftherioszisis commented May 10, 2022 •
edited
Loading

LGTM but why is the default set to False? I think the results from the old behavior are a bit hard to understand for a user who probably expects the results given by use_subtrees=True by default, WDT?

Default is False because this is the normal behavior as one would expect using entire trees of single types, i.e. the legacy behavior. I don't think we should break backward compatibility given that it is not necessary here.

Edit: However, making heterogeneous subtree processing the new standard is an enticing idea that would make things simpler.

@eleftherioszisis
Copy link
Contributor Author

@adrien-berchet may I merge this?

@adrien-berchet
Copy link
Member

Yep, let's keep this default value

@eleftherioszisis eleftherioszisis merged commit 5901eb2 into v4 May 12, 2022
@eleftherioszisis eleftherioszisis deleted the zisis/enable-use-subtrees-morph-stats branch May 12, 2022 10:24
eleftherioszisis added a commit that referenced this pull request Jun 7, 2022
@eleftherioszisis
Expose subtree processing from the morph_stats api (#1034)
eff04e8
@adrien-berchet adrien-berchet added this to the v4 milestone Feb 22, 2023
eleftherioszisis pushed a commit that referenced this pull request May 14, 2024
@mgeplf
V4 (#1111)
548e054 
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrien-berchet adrien-berchet adrien-berchet approved these changes

@mgeplf mgeplf Awaiting requested review from mgeplf

Assignees
No one assigned
Labels
version 4.0
Projects
None yet
Milestone
v4
Development

Successfully merging this pull request may close these issues.
3 participants
@eleftherioszisis @codecov-commenter @adrien-berchet