8000 Should we error on "not-recommended" behavior in static typing tests ? · Issue #16891 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

Should we error on "not-recommended" behavior in static typing tests ? #16891

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
Closed
anirudh2290 opened this issue Jul 17, 2020 · 4 comments · Fixed by #16917
Closed

Should we error on "not-recommended" behavior in static typing tests ? #16891

anirudh2290 opened this issue Jul 17, 2020 · 4 comments · Fixed by #16917
Labels
41 - Static typing

Comments

@anirudh2290
Copy link
Member

This discussion stems from here : #16622 (comment)

The numpy documentation says the following about instantiating the dtype object using "{'field1': ..., 'field2': ..., ...}":

"This usage is discouraged, because it is ambiguous with the other dict-based construction method. If you have a field called ‘names’ and a field called ‘formats’ there will be a conflict."

I think static typing is a good place to catch not just "wrong" types used, but also to catch early such discouraged usage in numpy. So, IMO we should have the type checker error on such behavior. WDYT ?
@anirudh2290 anirudh2290 mentioned this issue Jul 17, 2020
Merged
@rgommers
Copy link
Member

I agree with this. I think we had a similar discussion before. Rather be strict in type annotations to signal the code we want people to write, than to provide typing for every bit of valid code.

@BvB93
Copy link
Member
BvB93 commented Jul 17, 2020

I think static typing is a good place to catch not just "wrong" types used, but also to catch early such discouraged usage in numpy. So, IMO we should have the type checker error on such behavior. WDYT ?

I share your sentiment.
At the end of the day type hints are a set of guidelines; not strict rules. Considering the relevant dtype(...) syntax is already officially discouraged in the documentation I feel it would be good to reflect this in aforementioned guidelines as well.

@person142
Copy link
Member
person142 commented Jul 18, 2020 •
edited
Loading

I think we had a similar discussion before.

Some previous discussion:

numpy/numpy-stubs#12
numpy/numpy-stubs#41
numpy/numpy-stubs#66 (comment)

So far we've come down on the side of strictness; though I imagine decisions will continue to be made on a case-by-case basis. I think an important thing is documenting the decisions so that we don't end up discussing the same points endlessly, so far I have been doing that here:

https://github.com/numpy/numpy/blob/master/numpy/typing/__init__.py#L25

@anirudh2290 anirudh2290 mentioned this issue Jul 20, 2020
Merged
@anirudh2290
Copy link
Member Author

Thanks for the suggestion, I have added the doc changes here : #16917.

@BvB93 BvB93 linked a pull request Jul 21, 2020 that will close this issue
DOC: Add correctness vs strictness consideration for np.dtype #16917
Merged
BvB93 pushed a commit to BvB93/numpy that referenced this issue Jul 30, 2020
MAINT: Removed support for the `np.dtype({'field1': ..., 'field2': ..…
d7849ec 
…., ...})` syntax

Removed per the discussion in numpy#16891
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
41 - Static typing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

DOC: Add correctness vs strictness consideration for np.dtype anirudh2290/numpy
4 participants
@rgommers @anirudh2290 @person142 @BvB93
0