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

Ban obsolete morphology repair stage from API #171

Merged
merged 2 commits into from
Aug 3, 2017
Merged

Conversation

eile
Copy link
Contributor

@eile eile commented Aug 3, 2017

No description provided.

@eile eile force-pushed the stage branch 2 times, most recently from ca5015d to 0da04c1 Compare August 3, 2017 07:46
Stefan Eilemann added 2 commits August 3, 2017 09:47
Was only used for deprecated h5 v2 morphologies. Contract for all other file types was undefined and leaked through the API. New impl will read latest repair stage and will write repaired morphologies.
Copy link
Contributor

@hernando hernando left a comment

Choose a reason for hiding this comment

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

Looks OK, but may I ask why did you do this change?

dataset = _file.openDataSet("/" + _g_root + "/" + toString(stage) +
"/" + _d_points);
dataset = _file.openDataSet("/" + _g_root + "/" + _stage + "/" +
_d_points);
Copy link
Contributor

Choose a reason for hiding this comment

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

You have changed the default semantics here. Are you sure there's no tool using v2 morphologies that would be affected by this change?

Copy link
Contributor

@tribal-tec tribal-tec Aug 3, 2017

Choose a reason for hiding this comment

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

In any case this has to be documented/mentioned in the changelog.

@eile eile merged commit 0544681 into BlueBrain:master Aug 3, 2017
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