-
-
Notifications
You must be signed in to change notification settings - Fork 32
Ensure stubs are valid for Python 2 and fix running of tests #19
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import numpy as np | ||
|
||
array = np.array([1, 2]) | ||
|
||
# The @ operator is not in python 2 | ||
array @ array | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some people on 3.4 may want to run the test suite, so perhaps this shouldn't be used? Mypy defaults to using the python version of the executable used to run it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think enough people are still running Python 3.4 that we need to worry about this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could try to come up with a more generalized way of specifying what versions of python a given file can run on, or extend the mechanism so we have something like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to see this change be in a separate commit to the reformatting (but I do agree with the reformatting in general)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I am OK with minor reformatting inline with other changes. There are relatively few cases where I've wanted to look at changes with more granularity than that provided by pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to pull this out to a separate commit if you'd like :) In my day job we only do squash-and-merge, so I'm not used to thinking about having granular commits within a PR haha.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been doing squash-and-merge for most of the PRs to numpy-stubs, so I think it's fine to keep this as is.