Skip to content

DOC: run typing checks #51352

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

Merged
merged 6 commits into from
Feb 17, 2023
Merged

DOC: run typing checks #51352

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5584eab
DOC: run typing checks
twoertwein Feb 12, 2023
a2d50f0
update wording a bit
twoertwein Feb 13, 2023
364e302
reference python setup
twoertwein Feb 13, 2023
0c5d880
Merge remote-tracking branch 'upstream/main' into typing_doc
twoertwein Feb 16, 2023
0852912
reference pandas CI
twoertwein Feb 16, 2023
2c28c57
include url to workflows
twoertwein Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions doc/source/development/contributing_codebase.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,17 @@ pandas uses `mypy <http://mypy-lang.org>`_ and `pyright <https://github.com/micr

.. code-block:: shell

pre-commit run --hook-stage manual --all-files mypy
pre-commit run --hook-stage manual --all-files pyright
pre-commit run --hook-stage manual --all-files pyright_reportGeneralTypeIssues
# the following might fail if the installed pandas version does not correspond to your local git version
pre-commit run --hook-stage manual --all-files
pre-commit run --hook-stage manual --all-files stubtest

# if the above fails due to stubtest
SKIP=stubtest pre-commit run --hook-stage manual --all-files
in your active python environment.

in your activated python environment. A recent version of ``numpy`` (>=1.22.0) is required for type validation.
.. warning::

* It is essential that the versions of ``mypy`` and ``numpy`` are the same as used on main. Please see :ref:`how to setup the python environment <contributing.mamba>`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should mention numpy here as we don't specify a version of numpy in environment.yml or requirements-dev.txt.

Secondly, can we use this PR to address this comment: #51313 (comment) ?

The contributing.mamba section doesn't indicate how you should update your mamba environment when versions change. This has bitten me multiple times, so it would be good to put something in the docs there.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't use mamba/conda so I don't feel comfortable commenting on how to update the environment.

My simple/stupid approach: If mypy/pyright fails, I just check which numpy/mypy version main installs for the most recently merged PR and then pip install the same version :) Just matching the numpy/mypy version has so far always worked for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't use mamba/conda so I don't feel comfortable commenting on how to update the environment.

OK, maybe I will do a PR about that.

My simple/stupid approach: If mypy/pyright fails, I just check which numpy/mypy version main installs for the most recently merged PR and then pip install the same version :) Just matching the numpy/mypy version has so far always worked for me.

Yes, but you're smarter about these things than our average contributor! I've found these kinds of issues a PITA to figure out what is wrong with my environment.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still think we should delete numpy from this part of the doc.

Copy link
Member

Choose a reason for hiding this comment

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

Hi @Dr-Irv - what do you suggest this be replaced with if you don't want to mention numpy?

I also use the approach of checking which version was running in CI, and checking my environment matches that. I think that'd be fine to document

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @Dr-Irv - what do you suggest this be replaced with if you don't want to mention numpy?

My concern here is that the sentence is saying "what is used in main", but there is no specific version on main . The version is determined by what is used in CI. So I'm not exactly sure what to say.

I also use the approach of checking which version was running in CI, and checking my environment matches that. I think that'd be fine to document

Think of it from a first-time contributor point of view. How are they supposed to figure out what was running in CI when they haven't pushed anything yet? Documenting how to look in the CI (or somewhere else?) would be fine to add here.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, rephrasing "what is used in main" to something to the effect of "what was used in the latest CI build", and mentioning which CI job they should be looking at (i.e. the one which runs mypy)

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to describe how to navigate to the section of a pandas CI report where the versions are listed (not sure whether that is clear enough - probably not).


.. _contributing.ci:

Expand Down