Skip to content

Clarification to docs regarding initial sform/qform code values #576

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
merged 10 commits into from
Nov 11, 2017

Conversation

pauldmccarthy
Copy link
Contributor

[see #574 for background]

This PR adds a little bit of documentation to the Nifti1Pair class, and to the nibabel user documentation page on working with NIfTI images, clarifying how the sform_code and qform_code fields are initialised under different circumstances.

Copy link
Member

@matthew-brett matthew-brett left a comment

Choose a reason for hiding this comment

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

Small typo - but otherwise looks good - thanks.

``affine`` does not match the affine that is in the ``header``,
the ``affine`` will be used, but the ``sform_code`` and
``qform_code`` fields in the header will be re-initialised to
their default values.. This is performed on the basis that, if
Copy link
Member

Choose a reason for hiding this comment

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

Extra period values..

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.254% when pulling 041be2f on pauldmccarthy:unambiguify_nifti_docs into 2139ce0 on nipy:master.

@codecov-io
Copy link

codecov-io commented Nov 9, 2017

Codecov Report

Merging #576 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #576   +/-   ##
=======================================
  Coverage   94.32%   94.32%           
=======================================
  Files         177      177           
  Lines       24695    24695           
  Branches     2639     2639           
=======================================
  Hits        23294    23294           
  Misses        925      925           
  Partials      476      476
Impacted Files Coverage Δ
nibabel/nifti1.py 91.14% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2139ce0...188b194. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.254% when pulling d158f50 on pauldmccarthy:unambiguify_nifti_docs into 2139ce0 on nipy:master.

The sform and qform codes will be initialised to 2 (aligned) and 0 (unknown)
respectively:

>>> img.get_sform(coded=True)
Copy link
Member

Choose a reason for hiding this comment

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

>>> img.get_sform(coded=True) # doctest: +NORMALIZE_WHITESPACE

@effigies
Copy link
Member

effigies commented Nov 9, 2017

Oops. You got to the typo before me.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.254% when pulling 92a8acb on pauldmccarthy:unambiguify_nifti_docs into 2139ce0 on nipy:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.254% when pulling 92a8acb on pauldmccarthy:unambiguify_nifti_docs into 2139ce0 on nipy:master.

Copy link
Member

@matthew-brett matthew-brett left a comment

Choose a reason for hiding this comment

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

Last couple of nitpicks, sorry.

>>> new_data = np.random.random(n1_img.shape[:3])
>>> new_img = nib.nifti1.Nifti1Image(data, None, header=new_header)

Then the newly created image will inherit the same sform and qform codes that
Copy link
Member

Choose a reason for hiding this comment

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

Capital T for Then reads oddly.

Notes
-----

If both a ``header`` and an ``affine`` are specified, and the ``affine``
Copy link
Member

Choose a reason for hiding this comment

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

I think you want single backticks for the parameters header and affine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's a single backtick for markdown, but double backticks for restructured text - this renders fine as it is.

Copy link
Member

Choose a reason for hiding this comment

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

Single backticks in rst gives the default markup, which, in the case of the numpy docstring renderer, has the right markup for referring to parameters. So, single backticks to refer to parameters, double backticks for typewriter font ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aah ok - I am not all that familiar with numpydoc - will fix it now!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.254% when pulling a6b4d83 on pauldmccarthy:unambiguify_nifti_docs into 2139ce0 on nipy:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.254% when pulling 188b194 on pauldmccarthy:unambiguify_nifti_docs into 2139ce0 on nipy:master.

@matthew-brett
Copy link
Member

Great - thanks for your patience.

@matthew-brett matthew-brett merged commit 084dd1c into nipy:master Nov 11, 2017
@pauldmccarthy pauldmccarthy deleted the unambiguify_nifti_docs branch March 26, 2018 09:53
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.

5 participants