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

Code and documentation string cleanup #10

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Oct 19, 2015

This is an attempt to follow standard PEP8 code formatting conventions and change to the numpy docstring format as discussed in #8.

No changes to functionality have been made, but a few changes were not purely stylistic:

  1. replace assert statements with explicit exceptions (e.g. raise ValueError(...))
  2. remove any calls to reload in the examples
  3. For Python 3 compatibility, from __future__ import division, print_function, absolute_import is used throughout with // used to explicitly specify integer division where necessary.
  4. Replace a few uses of numpy matrices with numpy arrays. The current recommendation of numpy developers is to avoid use of the matrix class.

I ran the current examples successfully in both Python 3.4 and Python 2.7, except for a couple I did not have data for (see #9).

Ideally, the remaining two examples should be run with proper data to make sure nothing was inadvertently broken by these changes prior to merging.

@@ -2,4 +2,5 @@
ISMRMRD Python Tools
"""

__all__ = ["sense", "transform", "show", "simulation", "coils", "grappa", "ndarray_io"]
__all__ = ["sense", "transform", "show", "simulation", "coils", "grappa"
"ndarray_io"]
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bug. You're missing a comma between "grappa" and "ndarray_io".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks

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