Skip to content
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.

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

ENH: Add annotations for np.rec #19949

Merged
merged 8 commits into from
Sep 24, 2021
Merged

ENH: Add annotations for np.rec #19949

merged 8 commits into from
Sep 24, 2021

Conversation

BvB93
Copy link
Member

@BvB93 BvB93 commented Sep 24, 2021

This PR adds annotations for the np.rec functions as well as the classes that it exports to the main numpy namespace.

In general, the dtype produced by np.rec-based classes and functions can be specified in one of two ways:

  • Directly via the dtype argument.
  • With up to five helper arguments that operate via np.format_parser: formats, names, titles, aligned and byteorder

    numpy/numpy/core/records.py

    Lines 422 to 425 in e0d4bcf

    if dtype is not None:
    descr = sb.dtype(dtype)
    else:
    descr = format_parser(formats, names, titles, aligned, byteorder).dtype

These two approaches are currently typed as being mutually exclusive, i.e. if dtype is specified than one may not specify formats. While this mutual exclusivity is not (strictly) enforced during runtime, combining both dtype specifiers can lead to unexpected or even downright buggy behavior; enough reason to activelly discourage it, in my opinion.

@charris
Copy link
Member

charris commented Sep 24, 2021

@BvB93 OT: Note that LGTM has an alert for mypy_plugin.py, https://lgtm.com/projects/g/numpy/numpy/alerts/?mode=list&severity=error.

@BvB93
Copy link
Member Author

BvB93 commented Sep 24, 2021

Seems to be a false positive, the relevant branch of code can only be reached if the to-be raised exception is not None:

if t.TYPE_CHECKING or MYPY_EX is None:
    ...
else:
    def plugin(version: str) -> t.Type[_NumpyPlugin]:
        """An entry-point for mypy."""
        raise MYPY_EX  # LGTM: Illegal class 'NoneType' raised; will result in a TypeError being raised instead.

@charris charris merged commit 7b22284 into numpy:main Sep 24, 2021
@charris
Copy link
Member

charris commented Sep 24, 2021

Thanks Bas.

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

Successfully merging this pull request may close these issues.

None yet

2 participants