Skip to content

ENH: Improve the placeholder annotations within sub-modules (part 2)#18853

Merged
charris merged 6 commits intonumpy:mainfrom
BvB93:placeholder_main2
Apr 30, 2021
Merged

ENH: Improve the placeholder annotations within sub-modules (part 2)#18853
charris merged 6 commits intonumpy:mainfrom
BvB93:placeholder_main2

Conversation

@BvB93
Copy link
Copy Markdown
Member

@BvB93 BvB93 commented Apr 26, 2021

Xref #18838; follow up on #18842.

This PR improves the current Any-based placeholder annotations within a number of modules,
replacing them with explicit functions, classes or objects when appropiate. While parameters of the
respective methods and functions remain unannotated (for now), these changes nevertheless provides
a notable improvement over a plain Any.

Part 2 out of 3 in a series of PRs; updates the placeholder annotations of np.linalg and np.lib.

Comment on lines 6 to 11
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NDArrayOperatorsMixin is not technically an abstract baseclass, but in practice it is relient on subclasses implementing __array_ufunc__. Let me know if this should be changed or not.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: relient -> reliant

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed as of 7d6d74a.

@BvB93 BvB93 force-pushed the placeholder_main2 branch from 4d80321 to ee78c44 Compare April 26, 2021 14:54
ROOT / "__init__.pyi",
ROOT / "char.pyi",
ROOT / "ctypeslib.pyi",
ROOT / "emath.pyi",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment below says """Test if all ``.pyi`` files are properly installed.""". Based on that, the new (and renamed) .pyi files should probably be added here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say it'd be fairly redundant here.

The reason for introducing this test is that numpy has a number of sub-packages (e.g. numpy/lib/setup.py), each of which needs to recognize .pyi as valid data files. Since the main numpy/lib/__init__.pyi file is already tested in aforementioned test, adding these new .pyi file adds little value.,

Co-Authored-By: h-vetinari <h.vetinari@gmx.com>
@charris charris merged commit e424d3e into numpy:main Apr 30, 2021
@charris
Copy link
Copy Markdown
Member

charris commented Apr 30, 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.

3 participants