Skip to content

Revert "account for labels that are not in the prefix in the hoc_reader"#493

Merged
bgmeulem merged 6 commits intodevelopfrom
morph
Apr 22, 2026
Merged

Revert "account for labels that are not in the prefix in the hoc_reader"#493
bgmeulem merged 6 commits intodevelopfrom
morph

Conversation

@bgmeulem
Copy link
Copy Markdown
Collaborator

I believe there is some confusion (at least there is on my part) on the intended behavior of renaming hoc morphology labels.
My intended purpose was to keep the hoc label as it exists in the morphology file, unless explicitly specified otherwise.
The only way in which the user was allowed to specify otherwise, is to provide a mapping between labels that may appear in a hoc morphology (in full, to avoid confusion), and some desired rename.
E.g. the label "BasalDendrite" could be renamed to "Dendrite" by extending the hoc label map with "basaldendrite": "dendrite".
It was then up to the user to keep this map unambiguous.

The implementation in 208e888 is the other way around. Rather than looking in the hoc map to see if some label (in full) needs to be renamed, it looks through the label, and checks if parts of it match a hoc map key. This was not my intended use case for this hoc label map, as it leads to ambiguous definitions for e.g. "ApicalDendrite", as it will match both apical and dend. While this error is gracefully handled, I'd argue this should not produce an error in the first place. Morphology files should be allowed to specify "ApicalDendrite" as a morphology label, and the user should be able to rename labels like "dend" or "apical" to "Dendrite" and "ApicalDendrite" respectively.

I believe there is some confusion (at least there is on my part) on the intended behavior of renaming `hoc` morphology labels.
My intended purpose was to keep the hoc label as it exists in the morphology file, unless explicitly specified otherwise.
The only way in which the user was allowed to specify otherwise, is to provide a mapping between labels that may appear in a `hoc` morphology (in full, to avoid confusion), and some desired rename.
E.g. the label "BasalDendrite" could be renamed to "Dendrite" by extending the hoc label map with `"basaldendrite": "dendrite"`.
It was then up to the user to keep this map unambiguous.

The implementation in 208e888 is the other way around. Rather than looking in the hoc map to see if some label (in full) needs to be renamed, it looks through the label, and checks if parts of it match a hoc map key. This was not my intended use case for this hoc label map, as it leads to ambiguous definitions for e.g. `"ApicalDendrite"`, as it will match both `apical` and `dend`. While this error is gracefully handled, it should not produce an error in the first place. Morphology files should be allowed to specify "ApicalDendrite" as a morphology label, and the user should be able to rename labels like "dend" or "apical" to "Dendrite" and "ApicalDendrite" respectively.
@bgmeulem bgmeulem requested a review from su-saka April 22, 2026 10:56
@su-saka
Copy link
Copy Markdown
Collaborator

su-saka commented Apr 22, 2026

Yes good point, morphology files should be able to specify "ApicalDendrite", I see now that this method would interfere with the ability to leave the morphology files as is.
I'd suggest to then just adjust the regex to detect the label in full even if there is an underscore in there, e.i., "apical_dend_0_1" should return "apical_dend" and not "apical" as it currently does.

`hoc` labels are now parsed by considering any suffix that contains underscores and numbers only as the suffix. anything that comes before that is the prefix.
@bgmeulem
Copy link
Copy Markdown
Collaborator Author

ok the regex now considers anything string segment that contains only digits and underscores as the suffix. anything before that is the prefix. anything after the suffix is dropped, but shouldn't exist (?)

>>> _extract_label_and_name_from_hoc("dend_1_0_1")
('dend', 'dend_1_0_1')
>>> _extract_label_and_name_from_hoc("apical_dend_1_0_1")
('apical_dend', 'apical_dend_1_0_1')
>>> _extract_label_and_name_from_hoc("apical")
('apical', 'apical')
>>> _extract_label_and_name_from_hoc("Apical1_dend_1_0_1")
('Apical1_dend', 'Apical1_dend_1_0_1')

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 50.54%. Comparing base (4d0777c) to head (c42d144).
⚠️ Report is 37 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #493   +/-   ##
========================================
  Coverage    50.53%   50.54%           
========================================
  Files          249      249           
  Lines        21104    21107    +3     
========================================
+ Hits         10665    10668    +3     
  Misses       10439    10439           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bgmeulem bgmeulem merged commit facf86b into develop Apr 22, 2026
4 checks passed
@bgmeulem bgmeulem deleted the morph branch April 22, 2026 16:09
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