-
-
Notifications
You must be signed in to change notification settings - Fork 32
Add basic stubs for some class attributes #9
Conversation
numpy/__init__.pyi
Outdated
|
|
||
| def newbyteoder(self, new_order:str = ...): dtype | ||
|
|
||
| str: builtins.str |
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 segregated this here so that the other strs don't need to have builtins.str here (see python/mypy#3775).
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.
Please add this as a comment in the code
shoyer
left a comment
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.
This is a great start, thanks!
numpy/__init__.pyi
Outdated
| alignment: int | ||
| byteorder: str | ||
| char: str | ||
| descr: List[Tuple[str, str]] |
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.
Entries in descr can only be three element tuples, with the last item indicating a shape, e.g.,
>>> np.dtype([('x', 'f4'), ('y', np.float32), ('z', 'f4', (2,2))]).descr
[('x', '<f4'), ('y', '<f4'), ('z', '<f4', (2, 2))]
numpy/__init__.pyi
Outdated
| byteorder: str | ||
| char: str | ||
| descr: List[Tuple[str, str]] | ||
| fields: Optional[Mapping[str, Union[Tuple[dtype, int], Tuple[dtype, int, str]]]] |
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.
It looks like the final title field can have any type, so it should probably be keyed as Any instead of str: https://docs.scipy.org/doc/numpy-1.13.0/reference/generated/numpy.dtype.fields.html#numpy.dtype.fields
numpy/__init__.pyi
Outdated
|
|
||
| def newbyteoder(self, new_order:str = ...): dtype | ||
|
|
||
| str: builtins.str |
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.
Please add this as a comment in the code
numpy/__init__.pyi
Outdated
| subdtype: Optional[Tuple[dtype, Tuple[int, ...]]] | ||
|
|
||
|
|
||
| def newbyteoder(self, new_order:str = ...): dtype |
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.
please keep the space after : per pep8. This should be new_order: str = ...
numpy/__init__.pyi
Outdated
| subdtype: Optional[Tuple[dtype, Tuple[int, ...]]] | ||
|
|
||
|
|
||
| def newbyteoder(self, new_order:str = ...): dtype |
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.
please keep the space after : per pep8. This should be:
new_order: str = ...
numpy/__init__.pyi
Outdated
| type: builtins.type | ||
|
|
||
| @property | ||
| def base(self) -> dtype: ... |
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.
Is there a reason why this is a property vs. an attribute for everything else? Does mypy care about the difference?
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.
Properties can't be assigned to unless they have a setter. With this stub, if you had x: dtype, mypy would allow x.type = some_type but not x.base = some_dtype.
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.
OK, in that case we should also convert most of the other attributes into readonly properties.
numpy/__init__.pyi
Outdated
| _dtype = dtype # for ndarray type | ||
|
|
||
|
|
||
| class flagsobj: |
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.
flagsobj is not exposed by NumPy's public API, so let's give it a private name (_flagsobj?)
numpy/__init__.pyi
Outdated
| def base(self) -> dtype: ... | ||
|
|
||
|
|
||
| _dtype = dtype # for ndarray type |
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.
Give this a name like _dtype_class to make it a little clearer what it is?
numpy/__init__.pyi
Outdated
| data: memoryview | ||
| dtype: _dtype | ||
| flags: flagsobj | ||
| flat: Any |
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.
This is a flatiter object
| strides: Tuple[int, ...] | ||
| ctypes: _ctypes | ||
| base: Optional[ndarray] | ||
|
|
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.
This list is incomplete for now, so let's add def __getattr__(self, name) -> Any: ....
numpy/__init__.pyi
Outdated
| subdtype: Optional[Tuple[dtype, Tuple[int, ...]]] | ||
|
|
||
|
|
||
| def newbyteoder(self, new_order:str = ...): dtype |
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 assume this should be newbyteorder, not newbyteoder?
* Replace tab in setup.py with spaces * Add Travis * Add flake8 config to pass tests * Use `-` instead of `_` See python/peps#568 * Add Travis badge to README * Use numpy 1.14.0
|
This should be ready for another round of review! |
numpy/__init__.pyi
Outdated
| @property | ||
| def descr(self) -> List[Union[ | ||
| Tuple[str, str], | ||
| Tuple[str, str, Tuple[int, ...]]]]: ... |
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.
optional: it might be convenient to define an alias _Shape = Tuple[int, ...]
numpy/__init__.pyi
Outdated
| names: Optional[Tuple[str, ...]] | ||
| updateifcopy: bool | ||
| writeable: bool | ||
| writebackifcopy: bool |
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 not quite sure where these attributes came from -- I can't find any of updateifcopy, writeable and writebackifcopy on dtype.
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.
Actually, these are missing down below on flagsobj.
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.
Oops 😊
| def str(self) -> builtins.str: ... | ||
|
|
||
| @property | ||
| def type(self) -> builtins.type: ... |
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.
More specifically, we could define the np.generic class and switch this to Type[generic] (but this is also fine for now -- certainly let's save defining the full NumPy scalar type hierarchy for later).
|
Thanks @alanhdu ! |
| def type(self) -> builtins.type: ... | ||
|
|
||
|
|
||
| _dtype_class = dtype # for ndarray type |
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.
Why does this have a duplicate name?
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.
Otherwise mypy gets confused when you to type the dtype ndarray attribute, e.g., def dtype(self) -> dtype refers to ndarray.dtype for the return value, not the dtype class.
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.
Can you fix it with an import numpy as np, and then using np.dtype?
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 don't know exactly how mypy handles circular imports, but I suspect that importing a module from itself would not work.
No description provided.