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

CI: fix last docbuild warnings #25964

Merged
merged 6 commits into from
Mar 10, 2024
Merged

CI: fix last docbuild warnings #25964

merged 6 commits into from
Mar 10, 2024

Conversation

mattip
Copy link
Member

@mattip mattip commented Mar 8, 2024

Related to #13114. I am not seeing the warnings locally that I see in the CI run, so maybe updating the image will help?

Copy link
Member

@seberg seberg left a comment

Choose a reason for hiding this comment

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

No idea if it works (my guess is no), but even if not, we could just update it anyway.

@mattip mattip marked this pull request as draft March 8, 2024 12:06
@mattip
Copy link
Member Author

mattip commented Mar 8, 2024

Down to 16 warnings, 7 of them in the new release notes.

list of warnings
/home/circleci/.pyenv/versions/3.11.8/lib/python3.11/collections/abc.py:docstring of collections.abc.Mapping.get:1: WARNING: py:class reference target not found: D[k] if k in D, else d.  d defaults to None.
/home/circleci/.pyenv/versions/3.11.8/lib/python3.11/collections/abc.py:docstring of collections.abc.Mapping.items:1: WARNING: py:class reference target not found: a set-like object providing a view on D's items
/home/circleci/.pyenv/versions/3.11.8/lib/python3.11/collections/abc.py:docstring of collections.abc.Mapping.keys:1: WARNING: py:class reference target not found: a set-like object providing a view on D's keys
/home/circleci/.pyenv/versions/3.11.8/lib/python3.11/collections/abc.py:docstring of collections.abc.Mapping.values:1: WARNING: py:class reference target not found: an object providing a view on D's values
<unknown>:1: WARNING: py:class reference target not found: numpy.ma.core.MaskedArray
/home/circleci/repo/doc/source/reference/maskedarray.generic.rst:106:<autosummary>:1: WARNING: py:class reference target not found: numpy.ma.core.MaskedArray
/home/circleci/repo/doc/source/reference/routines.ma.rst:36:<autosummary>:1: WARNING: py:class reference target not found: numpy.ma.core.MaskedArray
/home/circleci/repo/doc/source/release/2.0.0-notes.rst:488: WARNING: undefined label: '_migration_c_descr'
/home/circleci/repo/doc/source/release/2.0.0-notes.rst:807: WARNING: py:obj reference target not found: numpy.linalg.vecdot
/home/circleci/repo/doc/source/release/2.0.0-notes.rst:1220: WARNING: py:obj reference target not found: numpy.fft._helper
/home/circleci/repo/doc/source/release/2.0.0-notes.rst:1229: WARNING: py:obj reference target not found: numpy.linalg._linalg
/home/circleci/repo/doc/source/release/notes-towncrier.rst:7: WARNING: py:obj reference target not found: numpy.polynomial.Polynomial
/home/circleci/repo/doc/source/release/notes-towncrier.rst:7: WARNING: py:obj reference target not found: numpy.polynomial.Polynomial
/home/circleci/repo/doc/source/release/notes-towncrier.rst:7: WARNING: py:obj reference target not found: numpy.polynomial.Polynomial
/home/circleci/repo/doc/source/user/how-to-io.rst:330: WARNING: py:meth reference target not found: pandas.DataFrame.to_numpy

. I don't understand the ones that come from NpzFile inheriting from Mapping and then the autosummary directive to document it

.. autosummary::
:toctree: generated/
load
save
savez
savez_compressed
lib.npyio.NpzFile

The generated docs then pick up the get, items, keys, values methods from the Mapping class and try to document them, but this ends up messed up and emitting warnings. The generated reference from our template for numpy.lib.npyio.NpzFile.rst is

numpy.lib.npyio.NpzFile
=======================

.. currentmodule:: numpy.lib.npyio

.. autoclass:: NpzFile

   

   .. HACK -- the point here is that we don't want this to appear in the output, but the autosummary should still generate the pages.
      .. autosummary::
         :toctree:
      
         NpzFile.close
         NpzFile.get
         NpzFile.items
         NpzFile.keys
         NpzFile.values



   

   .. HACK -- the point here is that we don't want this to appear in the output, but the autosummary should still generate the pages.
      .. autosummary::
         :toctree:
      
         NpzFile.fid
         NpzFile.zip

and for instance, NpysFile.get.rst is

:orphan:

numpy.lib.npyio.NpzFile.get
===========================

.. currentmodule:: numpy.lib.npyio

method

.. automethod:: numpy::lib.npyio.NpzFile.get

@tk0miya any thoughts what might be going wrong?

@mattip mattip changed the title CI: update docs buildimage CI: fix last docbuild warnings Mar 10, 2024
@mattip
Copy link
Member Author

mattip commented Mar 10, 2024

Gahh. It turns out the docstrings for Mapping look like

D.get(k[,d]) -> D[k] if k in D, else d.  d defaults to None.

The -> apparently tells sphinx that this is a type hints, and so it tries to look up a type for D[k] if k in D, else d. d defaults to None. which fails, that is obviously not a type.

For this PR I will add new docstrings to NpzFile.get and friends, but it would be nice to solve this more systematically. I think the fix should come from CPython, that docstring looks wrong.

@mattip mattip marked this pull request as ready for review March 10, 2024 14:05
@mattip
Copy link
Member Author

mattip commented Mar 10, 2024

Marking as ready for review (assuming CI passes) and rebased to run tests since I changed the implementation of NpyzFile

@mattip
Copy link
Member Author

mattip commented Mar 10, 2024

Closes #13114, whohoo!

@charris charris merged commit 1ab8806 into numpy:main Mar 10, 2024
55 of 61 checks passed
@charris
Copy link
Member

charris commented Mar 10, 2024

Thanks Matti.

@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Mar 10, 2024
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