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

Deprecate manifold in favor of pixel_origin and pixel_deltas #305

Merged
merged 4 commits into from
Oct 7, 2019

Conversation

rly
Copy link
Contributor

@rly rly commented Aug 30, 2019

Implementing spec changes from NeurodataWithoutBorders/pynwb#576

ajtritt
ajtritt previously approved these changes Sep 17, 2019
@bendichter
Copy link
Contributor

bendichter commented Oct 7, 2019

@rly can we merge this?

@rly
Copy link
Contributor Author

rly commented Oct 7, 2019

@bendichter I have been hung up on the names of the replacement datasets. To me, they are not intuitive enough and "pixel" is not appropriate for 3D data. What do you think about "origin_coords" and "grid_spacing" instead?

@bendichter
Copy link
Contributor

@rly Yeah, "origin_coords" and "grid_spacing" are good names

@rly rly requested review from ajtritt, bendichter and oruebel October 7, 2019 22:25
@oruebel
Copy link
Contributor

oruebel commented Oct 7, 2019

@rly @bendichter I assume the idea is to then also rename pixel_origin and pixel_deltas in NeurodataWithoutBorders/pynwb#576? Correct?

@bendichter
Copy link
Contributor

@oruebel that's right. I think it will probably be easier to re-do that PR given how many merge conflicts there are

@oruebel
Copy link
Contributor

oruebel commented Oct 7, 2019

Sounds good.

@rly rly merged commit e813662 into dev Oct 7, 2019
@rly rly deleted the enh/deprecate_manifold branch October 7, 2019 23:33
@rly
Copy link
Contributor Author

rly commented Oct 7, 2019

@bendichter can you make a new PR to replace NeurodataWithoutBorders/pynwb#576 ?

@bendichter
Copy link
Contributor

sure

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.

4 participants