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.

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

Support morphology containers #274

Merged
merged 3 commits into from
Oct 31, 2024
Merged

Support morphology containers #274

merged 3 commits into from
Oct 31, 2024

Conversation

mgeplf
Copy link
Contributor

@mgeplf mgeplf commented Oct 30, 2024

  • use the morphio.Container, to both load from a directory or from a file

@mgeplf mgeplf force-pushed the handle-container-morphs branch from 0291bb1 to 76b6c67 Compare October 30, 2024 12:57
* when the `alternative-morphs` has an `h5` version
  use the `morphio.Container`, to both load from a directory of from a
  file
@mgeplf mgeplf force-pushed the handle-container-morphs branch from 76b6c67 to 3fb37a7 Compare October 30, 2024 13:01
Copy link
Member

@matz-e matz-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

CHANGELOG.rst Show resolved Hide resolved
bluepysnap/morph.py Outdated Show resolved Hide resolved
if extension == "h5" and Path(morph_dir).is_file():
raise BluepySnapError(
f"'{morph_dir}' is a morphology container, so a directory does not exist"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the circuit is using a h5 container instead of a directory, I wonder if we want to raise an error anytime that get_morphology_dir or get_filepath are called, regardless of the extension specified as a parameter?
Probably in the medium/long term those functions could be deprecated, but need to check in what use cases they are needed at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean if it's the only alternaive-morphology?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but after checking better it's already good as is, because the container file is retrieved only with extension=h5

Comment on lines +139 to +143
with pytest.raises(BluepySnapError):
test_obj.get_morphology_dir(extension="h5")

with pytest.raises(BluepySnapError):
test_obj.get_filepath(node_id, extension="h5")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but the specific reason of BluepySnapError could be checked with match=...

@mgeplf mgeplf merged commit aeeb1f2 into master Oct 31, 2024
5 checks passed
@mgeplf mgeplf deleted the handle-container-morphs branch October 31, 2024 14:19
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.

3 participants