Skip to content

MRG: rewrite gifti docstrings; refactor init #407

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

Merged

Conversation

matthew-brett
Copy link
Member

Responding to comments on Ben's previous PR #403.

Expand and fix docstrings and gifti attribute descriptions from gifti
spec.

Rewrite GiftiDataArray init to allow full initialization of object, that
can be used from GiftiDataArray.from_array. Add tests.

Fix up the tests - there were some duplicate test names so some tests were not being run.

Remove unused label attribute of GiftiLabel object.

@matthew-brett matthew-brett force-pushed the refactor-gifti-init-and-docstrings branch from 6095e62 to 4c1db3b Compare February 13, 2016 20:55
Responding to comments on Ben's previous PR nipy#403.

Expand and fix docstrings and gifti attribute descriptions from gifti
spec.

Rewrite GiftiDataArray init to allow full initialization of object, that
can be used from GiftiDataArray.from_array.

The refactor of the init needs tests; I'll add these if y'all agree this
init refactoring is the right thing to do.
Fix up a couple of duplicate test names.

Remove test of setting string to DataArray - it's not allowed in the
spec.

Add tests for dataarray initialization.
Raise a specific error when parsing a gifti file that does not conform
to the gifti spec.
@matthew-brett matthew-brett force-pushed the refactor-gifti-init-and-docstrings branch from fcdb5a8 to 7f965d7 Compare February 14, 2016 01:54
@matthew-brett matthew-brett changed the title WIP: rewrite gifti docstrings; refactor init MRG: rewrite gifti docstrings; refactor init Feb 14, 2016
"""

def __init__(self, key=0, label='', red=None, green=None, blue=None,
alpha=None):
self.key = key
self.label = label
Copy link
Member

Choose a reason for hiding this comment

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

Guessing you're removing this because GiftiLabel().label isn't referenced anywhere? If so, should it still be in the parameter list?

@effigies
Copy link
Member

Didn't get to the tests, but the docstrings look good.

Does GiftiDataArray.from_array make sense to deprecate, now that it simply passes through to __init__?

`label` doesn't appear in the spec, and our code doesn't use it either.
@matthew-brett matthew-brett force-pushed the refactor-gifti-init-and-docstrings branch from c1fe413 to c2a8f52 Compare February 14, 2016 03:34
@matthew-brett
Copy link
Member Author

Yes, I guess we could deprecate from_array...

Deprecate GiftiDataArray.from_array in favor of GiftiDataArray
constructor.
@effigies
Copy link
Member

I read through the tests. Looks reasonable.

I think this refactor of __init__ makes sense, but somebody who has actually used Gifti might have other opinions.

@matthew-brett
Copy link
Member Author

Ben - any comments here?

@matthew-brett
Copy link
Member Author

OK - will merge this one tomorrow unless I hear otherwise.

matthew-brett added a commit that referenced this pull request Feb 27, 2016
…cstrings

MRG: rewrite gifti docstrings; refactor init

Responding to comments on Ben's previous PR #403.

Expand and fix docstrings and gifti attribute descriptions from gifti
spec.

Rewrite `GiftiDataArray` init to allow full initialization of object, that
can be used from `GiftiDataArray.from_array`.  Deprecate `GiftiDataArray.from_array` as redundant.

Add tests.

Fix up the tests - there were some duplicate test names so some tests were not being run.

Remove unused `label` attribute of `GiftiLabel` object.
@matthew-brett matthew-brett merged commit d96945e into nipy:master Feb 27, 2016
@effigies effigies mentioned this pull request Mar 15, 2016
4 tasks
grlee77 pushed a commit to grlee77/nibabel that referenced this pull request Mar 15, 2016
…d-docstrings

MRG: rewrite gifti docstrings; refactor init

Responding to comments on Ben's previous PR nipy#403.

Expand and fix docstrings and gifti attribute descriptions from gifti
spec.

Rewrite `GiftiDataArray` init to allow full initialization of object, that
can be used from `GiftiDataArray.from_array`.  Deprecate `GiftiDataArray.from_array` as redundant.

Add tests.

Fix up the tests - there were some duplicate test names so some tests were not being run.

Remove unused `label` attribute of `GiftiLabel` object.
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.

2 participants