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

DOC: Add `Imports` section on package shorthands recommendations. #1716

Merged
merged 1 commit into from Feb 4, 2019

Conversation

@jhlegarreta
Copy link
Contributor

commented Jan 4, 2019

Add an Imports section to the style guideline on package shorthands
recommendations.

@jhlegarreta

This comment has been minimized.

Copy link
Contributor Author

commented Jan 4, 2019

Other suggestions are welcome.

@codecov-io

This comment has been minimized.

Copy link

commented Jan 5, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@51234a4). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #1716   +/-   ##
========================================
  Coverage          ?   84.2%           
========================================
  Files             ?     115           
  Lines             ?   13566           
  Branches          ?    2140           
========================================
  Hits              ?   11423           
  Misses            ?    1645           
  Partials          ?     498
@arokem

This comment has been minimized.

Copy link
Member

commented Jan 6, 2019

@jhlegarreta

This comment has been minimized.

Copy link
Contributor Author

commented Jan 6, 2019

I do share that, far more clear than nb. If other strong preferences exist throughout DIPY, please go ahead and comment.

@skoudoro

This comment has been minimized.

Copy link
Member

commented Jan 7, 2019

I think you can add all default requirements scipy as sp and no alias for h5py and cython

@jhlegarreta jhlegarreta force-pushed the jhlegarreta:AddImportSectionToStyleGuide branch from eb96802 to 94bda01 Jan 7, 2019

@jhlegarreta

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2019

cython is never imported from a Python file, right? For the Cython cases, PR #1714 adds the appropriate section.

Other cases I found and may potentially be added (with modifications if needed):

import os.path as op
import scipy.sparse as sps
import scipy.linalg as la
@skoudoro

This comment has been minimized.

Copy link
Member

commented Feb 1, 2019

cython is never imported from a Python file, right?

Yes, you are right

Other cases I found and may potentially be added

IMHO, I do not think you should add those

@jhlegarreta

This comment has been minimized.

Copy link
Contributor Author

commented Feb 1, 2019

Then we're done with this and it can be merged.

@skoudoro
Copy link
Member

left a comment

After your small correction, I will merge this PR this Sunday if there is no other comment

doc/devel/coding_style_guideline.rst Outdated Show resolved Hide resolved
DOC: Add `Imports` section on package shorthands recommendations.
Add an `Imports` section to the style guideline on package shorthands
recommendations.

@jhlegarreta jhlegarreta force-pushed the jhlegarreta:AddImportSectionToStyleGuide branch from 94bda01 to 3130522 Feb 2, 2019

@skoudoro skoudoro merged commit cfa1ed1 into nipy:master Feb 4, 2019

4 checks passed

codecov/patch Coverage not affected.
Details
codecov/project No report found to compare against
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.