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

Clean-up Morphology Loaders #1981

Closed
8 tasks done
thorstenhater opened this issue Sep 16, 2022 · 0 comments · Fixed by #2218
Closed
8 tasks done

Clean-up Morphology Loaders #1981

thorstenhater opened this issue Sep 16, 2022 · 0 comments · Fixed by #2218
Labels
arbor IO IO support in arborio code quality Feature Request good 1st issue help wanted interface Anything user-facing, including public C++ and Python APIs.

Comments

@thorstenhater
Copy link
Contributor

thorstenhater commented Sep 16, 2022 •
edited
Loading

Currently we have three kinds of loader as categorised by their return value

  • NML returns a really complex structure containing
    • all labels and regions declared by the input
    • a list of morphologies declared in <cell> tags
    • a list of morphologies declared in <morphologies>
  • ASC returns a structure containing the morphology and a fixed set of labels
  • SWC returns a bare morphology and we have a list of predefined labels
  • ASC and SWC can be loaded as segment_trees, but not NML

This is neither conforming to the least surprise principle nor ergonomic. Thus

  • SWC should return a structure similar to the current ASC loader (eg labelled_morphology, but maybe a pair is fine as well); subsuming the external tags
  • ASC should insert custom tags (these are currently parsed but discarded)
    • ASC returns a list of markers and spines.
  • NML should be a simpler structure to handle
    • <cell> and <morphology> can be unified (but still tagged by the origin),
    • add segment_tree loading
    • asking for a specific morphology in an nml object should give back a labelled_morphology
  • Finally: cable_cell should add a new constructor consuming a labelled_morphology and a decor
    • changed my mind here: it doesn't make the API any nicer and thus complicates matters.
  • Bonus: add convenience function load_morphology dispatching based on file suffix
    • doesn't work since NML and asc+swc return fundamentally different objects
@thorstenhater thorstenhater added help wanted code quality arbor IO IO support in arborio good 1st issue interface Anything user-facing, including public C++ and Python APIs. Feature Request labels Sep 16, 2022
This was referenced Oct 19, 2022
Merged
Open
@thorstenhater thorstenhater mentioned this issue Sep 11, 2023
Merged
thorstenhater added a commit that referenced this issue Mar 5, 2024
@thorstenhater
Unify loading of Standard Morphology Files. (#2218)
08a157f 
All morphology loaders now return a `loaded_morphology` object
comprising
- `morphology`: the morphology corresponding to the data on disk
- `segment_tree`: the raw segment tree
- `label_dict`: all labels defined in the file
- `asc`/`swc`: `axon`, `soma`, `dend`, and `apic` based on tag 1, 2, 3,
4.
- `nml`: the union of the label dicts contained in the metadata,
prefixed by `grp:`, `nmd:`, and `seg:`.
- `metadata`: a set of items given by the fileformat
  - `swc`: nothing
  - `asc`: a list of spines and marker sets
  - `nml`: 
    - `named_segments` a label dict
    - `segment_groups` a label dict `name` -> segment ids
    - `group_segments` a map from segment id to group name(s)
    - `segments` a label dict, one entry per id
    - `cell_id` id of the cell defining this morphology, if any
    - `id` id of the morphology

Note that loading neuroml still goes via the `neuroml` object and the
`cell_morphology`/`morphology`
accessors return an optional `loaded_morphology`.

## ⚠️ Breaking ⚠️ 

- Removed the `load_*_raw` functions, as `loaded_morphology` bundles the
segment tree.
- The interface of `load_*` now has an extra indirection.
- The interface of `neuroml` changed.
- `label_dict::import` is now called `extend` for future safety (=C++
modules) and Python; calling things `import` (=a keyword) is not well
received by `black` et al.

## Issues

Closes #1981
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
arbor IO IO support in arborio code quality Feature Request good 1st issue help wanted interface Anything user-facing, including public C++ and Python APIs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Unify loading of Standard Morphology Files. thorstenhater/arbor
1 participant
@thorstenhater