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

[nixio] Use Neo object names in NIX file #551

Merged
merged 3 commits into from Jul 20, 2018

Conversation

achilleas-k
Copy link
Contributor

Last year I changed the NixIO to use UUIDs as names for objects as a way to get around a couple of issues. Issue #311 is where the discussion happened and #331 is the relevant PR where this was implemented.

Some time later, there was a discussion on the mailing list regarding the write performance but, as a side topic, the issue of the naming also came up. In short, with the current naming scheme, the NIX/HDF5 file that is generated has names that make it difficult to work with and can be a hindrance (and an annoyance) for people who purposefully name their objects in order to be able to navigate a file.

This PR adds a new feature which somewhat backtracks from the design decision of automatically naming objects. The write_all_blocks() and write_block() methods now accept an optional, boolean argument, use_obj_names (false, by default). If enabled, the objects in the NIX file will use the names of the corresponding Neo objects. Before writing, however, the whole Neo object tree is checked for conflicts, based on the requirements of NIX and HDF5. If a conflict is found, a ValueError is raised and no objects are written.

I felt it was important to not check the names while writing and instead check names beforehand. This avoids potential issues with partially writing data with names that later turn out to have conflicts. It's also good for avoiding situations where a write will fail half way through the saving of a large file that could take minutes (or hours). I've clocked the name checking function and even with thousands of objects (the saving of which took over 20 mins), it only needs a few seconds to run.

Tests for the new functionality are also included.

This option triggers a check to determine if the Neo names satisfy the
uniqueness requirement for NIX and if the check passes, the Neo objects
are annotated with their own name (as nix_name) to make the writer use
that name in NIX.
@coveralls
Copy link

coveralls commented Jul 18, 2018

Coverage Status

Coverage decreased (-6.3%) to 49.623% when pulling 6e7f557 on G-Node:nixio-use-neo-names into 7753fb2 on NeuralEnsemble:master.

@JuliaSprenger
Copy link
Member

Thanks for the improvements. I tested it and it works.
I will merge this now.

@JuliaSprenger JuliaSprenger merged commit 9ec44c6 into NeuralEnsemble:master Jul 20, 2018
@achilleas-k achilleas-k deleted the nixio-use-neo-names branch September 21, 2018 09:49
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.

None yet

3 participants