8000 ENH: Strict checking of ufunc keyword argument names by jaimefrio · Pull Request #5659 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

ENH: Strict checking of ufunc keyword argument names #5659

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 12, 2015
Merged

ENH: Strict checking of ufunc keyword argument names #5659

merged 1 commit into from
Mar 12, 2015

Conversation

jaimefrio
Copy link
Member

Raises a TypeError if any of the keyword arguments supplied to a
ufunc does not exactly match the name in the signature. Prior to
this, trailing characters were discarded, e.g. 'out2' would be
treated as if it where 'out'.

@jaimefrio
ENH: Strict checking of ufunc keyword argument names
3a39a0e 
Raises a TypeError if any of the keyword arguments supplied to a
ufunc does not exactly match the name in the signature. Prior to
this, trailing characters were discarded, e.g. 'out2' would be
treated as if it where 'out'.
@jaimefrio
Copy link
Member Author

Some explanation on the sig / signature thing: setting the keyword strictly to sig causes over 100 test errors and failures. The main culprit is umath_linalg, which extensively uses signature to call what the docs say should be sig. For now I have decided to embrace the dual keyword reality we already live in.

If there is agreement I have no problem going back to sig only and fixing all the uses of signature in the numpy code base. My real worry is what if signature has leaked outside, and enforcing what the docs say breaks third party code. Their bad for using undocumented features, but still...

@jaimefrio jaimefrio mentioned this pull request Mar 10, 2015
Merged
@mhvk
Copy link
Contributor
mhvk commented Mar 10, 2015

Nice! And I think it is OK to support both sig and signature (think explicit full names are usually the better choice anyway...).

@ewmoore
Copy link
Contributor
ewmoore commented Mar 10, 2015

How does this play with the override mechanism? Can they both appear or is
this normalized to one or the other there?

On Tue, Mar 10, 2015 at 9:23 AM, Marten van Kerkwijk <
notifications@github.com> wrote:

Nice! And I think it is OK to support both sig and signature (think
explicit full names are usually the better choice anyway...).

—
Reply to this email directly or view it on GitHub
#5659 (comment).

@jaimefrio
Copy link
Member Author

I also like signature better than sig, and even tried, just for fun, to see what would happen if we enforced it to be signature: a single, easy to fix error in the numpy tests. But this would really be a non backwards compatible API change, so I really think we need a better justification than "me likey!"

Good question, Eric, will look into it. I think the only thing the override does with the arguments is to convert the outputs into a tuple and add them to the keyword dictionary. So behavior would be unchanged there: you could be receiving either sig or signature today.

@jaimefrio
Copy link
Member Author

So I just checked, @ewmoore, and things are like I thought: with this change the override mechanism may hand a dictionary with either sig or signature, before this change, anything starting with sig was a possibility. Do you think we should normalize it? To sig or to signature? Anyone actually using this has an opinion?

@njsmith
Copy link
Member
njsmith commented Mar 11, 2015

If we're picking just one then full words are generally nice things.
On Mar 10, 2015 8:42 PM, "Jaime" notifications@github.com wrote:

So I just checked, @ewmoore https://github.com/ewmoore, and things are
like I thought: with this change the override mechanism may hand a
dictionary with either sig or signature, before this change, anything
starting with sig was a possibility. Do you think we should normalize it?
To sig or to signature? Anyone actually using this has an opinion?

—
Reply to this email directly or view it on GitHub
#5659 (comment).

@mhvk
Copy link
Contributor
mhvk commented Mar 11, 2015

👍 to passing on signature in full (but accepting sig).

@ewmoore
Copy link
Contributor
ewmoore commented Mar 11, 2015

I haven't written anything using the override, but, I think it absolutely
should be normalized to a standard value. Why make every override user do
it when we could do it in one place? +1 on passing signature.

On Wed, Mar 11, 2015 at 9:21 AM, Marten van Kerkwijk <
notifications@github.com> wrote:

[image: 👍] to passing on signature in full (but accepting sig).

—
Reply to this email directly or view it on GitHub
#5659 (comment).

@charris
Copy link
Member
charris commented Mar 12, 2015

+1 on normalizing override to pass "signature".

charris added a commit that referenced this pull request Mar 12, 2015
@charris
Merge pull request #5659 from jaimefrio/strict_out_check
27c68a3 
ENH: Strict checking of ufunc keyword argument names
@charris charris merged commit 27c68a3 into numpy:master Mar 12, 2015
@charris
Copy link
Member
charris commented Mar 12, 2015

LGTM, thanks Jaime.

7EB5

@jaimefrio jaimefrio deleted the strict_out_check branch March 12, 2015 21:36
@jaimefrio jaimefrio mentioned this pull request Mar 12, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
01 - Enhancement component: numpy._core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jaimefrio @mhvk @ewmoore @njsmith @charris
0