Skip to content
This repository was archived by the owner on Jun 10, 2020. It is now read-only.

Add basic stubs for some class attributes #9

Merged
merged 10 commits into from
Feb 19, 2018
Merged

Conversation

alanhdu
Copy link
Contributor

@alanhdu alanhdu commented Feb 18, 2018

No description provided.


def newbyteoder(self, new_order:str = ...): dtype

str: builtins.str
Copy link
Contributor Author

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).

Copy link
Member

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

Copy link
Member

@shoyer shoyer left a 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!

alignment: int
byteorder: str
char: str
descr: List[Tuple[str, str]]
Copy link
Member

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))]

byteorder: str
char: str
descr: List[Tuple[str, str]]
fields: Optional[Mapping[str, Union[Tuple[dtype, int], Tuple[dtype, int, str]]]]
Copy link
Member

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


def newbyteoder(self, new_order:str = ...): dtype

str: builtins.str
Copy link
Member

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

subdtype: Optional[Tuple[dtype, Tuple[int, ...]]]


def newbyteoder(self, new_order:str = ...): dtype
Copy link
Member

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 = ...

subdtype: Optional[Tuple[dtype, Tuple[int, ...]]]


def newbyteoder(self, new_order:str = ...): dtype
Copy link
Member

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 = ...

type: builtins.type

@property
def base(self) -> dtype: ...
Copy link
Member

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?

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.

Copy link
Member

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.

_dtype = dtype # for ndarray type


class flagsobj:
Copy link
Member

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?)

def base(self) -> dtype: ...


_dtype = dtype # for ndarray type
Copy link
Member

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?

data: memoryview
dtype: _dtype
flags: flagsobj
flat: Any
Copy link
Member

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

shape: Tuple[int, ...]
strides: Tuple[int, ...]
ctypes: _ctypes
base: Optional[ndarray]

Copy link
Member

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: ....

subdtype: Optional[Tuple[dtype, Tuple[int, ...]]]


def newbyteoder(self, new_order:str = ...): dtype

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?

Alan Du and others added 3 commits February 18, 2018 08:57
* 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
@alanhdu
Copy link
Contributor Author

alanhdu commented Feb 18, 2018

This should be ready for another round of review!

@property
def descr(self) -> List[Union[
Tuple[str, str],
Tuple[str, str, Tuple[int, ...]]]]: ...
Copy link
Member

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, ...]

names: Optional[Tuple[str, ...]]
updateifcopy: bool
writeable: bool
writebackifcopy: bool
Copy link
Member

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.

Copy link
Member

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.

Copy link
Contributor Author

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: ...
Copy link
Member

@shoyer shoyer Feb 19, 2018

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).

@shoyer shoyer merged commit 6166b30 into numpy:master Feb 19, 2018
@shoyer
Copy link
Member

shoyer commented Feb 19, 2018

Thanks @alanhdu !

@alanhdu alanhdu deleted the stubs branch February 19, 2018 20:13
def type(self) -> builtins.type: ...


_dtype_class = dtype # for ndarray type
Copy link
Member

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?

Copy link
Member

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.

Copy link
Member

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?

Copy link
Member

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants