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

Fix internal links in docs #1031

Merged
merged 19 commits into from Jan 19, 2024
Merged

Fix internal links in docs #1031

merged 19 commits into from Jan 19, 2024

Conversation

stephprince
Copy link
Contributor

@stephprince stephprince commented Jan 17, 2024

Motivation

See related PR in NeurodataWithoutBorders/pynwb#1827

There are some broken internal links in the hdmf documentation that impact pynwb as well.

How to test the behavior?

To see linkcheck warnings:

sphinx-build -n -b linkcheck ./docs/source ./docs/_build

To see updated docs:

cd docs
make html

Checklist

  • Did you update CHANGELOG.md with your changes?
  • Does the PR clearly describe the problem and the solution?
  • Have you reviewed our Contributing Guide?
  • Does the PR use "Fix #XXX" notation to tell GitHub to close the relevant issue numbered XXX when the PR is merged?

Copy link

codecov bot commented Jan 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b4bdcef) 88.50% compared to head (4811e5a) 88.51%.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #1031   +/-   ##
=======================================
  Coverage   88.50%   88.51%           
=======================================
  Files          45       45           
  Lines        9582     9586    +4     
  Branches     2724     2725    +1     
=======================================
+ Hits         8481     8485    +4     
  Misses        778      778           
  Partials      323      323           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stephprince stephprince marked this pull request as ready for review January 17, 2024 20:10
@stephprince stephprince requested a review from rly January 17, 2024 20:10
@rly
Copy link
Contributor

rly commented Jan 17, 2024

Here are the warnings that I see when running this locally:

/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5_utils.py:docstring of hdmf.backends.hdf5.h5_utils.HDF5IODataChunkIteratorQueue.append:1: WARNING: py:class reference target not found: Dataset
/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5_utils.py:docstring of hdmf.backends.hdf5.h5_utils.H5RegionSlicer:4: WARNING: py:class reference target not found: h5py.RegionReference
/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5_utils.py:docstring of hdmf.backends.hdf5.h5_utils.H5DataIO.filter_available:1: WARNING: py:class reference target not found: String
/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5tools.py:docstring of hdmf.backends.hdf5.h5tools.HDF5IO:1: WARNING: py:class reference target not found: Intracomm
/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5tools.py:docstring of hdmf.backends.hdf5.h5tools.docval.<locals>.dec.<locals>.func_call:1: WARNING: py:class reference target not found: Intracomm
/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5tools.py:docstring of hdmf.backends.hdf5.h5tools.docval.<locals>.dec.<locals>.func_call:1: WARNING: py:class reference target not found: Group
/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5tools.py:docstring of hdmf.backends.hdf5.h5tools.docval.<locals>.dec.<locals>.func_call:1: WARNING: py:class reference target not found: Link
/Users/rly/Documents/NWB/hdmf/src/hdmf/backends/hdf5/h5tools.py:docstring of hdmf.backends.hdf5.h5tools.docval.<locals>.dec.<locals>.func_call:1: WARNING: py:class reference target not found: Dataset
/Users/rly/Documents/NWB/hdmf/src/hdmf/build/builders.py:docstring of hdmf.build.builders.DatasetBuilder:6: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/build/builders.py:docstring of hdmf.build.builders.RegionBuilder:4: WARNING: py:class reference target not found: h5py.RegionReference
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/__init__.py:docstring of hdmf.common.docval.<locals>.dec.<locals>.func_call:1: WARNING: py:class reference target not found: Intracomm
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/alignedtable.py:docstring of hdmf.common.alignedtable.AlignedDynamicTable:17: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/alignedtable.py:docstring of hdmf.common.alignedtable.AlignedDynamicTable:21: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/alignedtable.py:docstring of hdmf.common.alignedtable.AlignedDynamicTable:27: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/alignedtable.py:docstring of hdmf.common.alignedtable.AlignedDynamicTable.category_tables:3: WARNING: py:class reference target not found: DynamicTables to be added to the container. NOTE
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/alignedtable.py:docstring of hdmf.common.alignedtable.docval.<locals>.dec.<locals>.func_call:10: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/alignedtable.py:docstring of hdmf.common.alignedtable.docval.<locals>.dec.<locals>.func_call:23: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/alignedtable.py:docstring of hdmf.common.alignedtable.docval.<locals>.dec.<locals>.func_call:25: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/resources.py:docstring of hdmf.common.resources.KeyTable:6: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/resources.py:docstring of hdmf.common.resources.EntityTable:6: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/resources.py:docstring of hdmf.common.resources.FileTable:6: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/resources.py:docstring of hdmf.common.resources.ObjectTable:6: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/resources.py:docstring of hdmf.common.resources.ObjectKeyTable:6: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/resources.py:docstring of hdmf.common.resources.EntityKeyTable:6: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/resources.py:docstring of hdmf.common.resources.docval.<locals>.dec.<locals>.func_call:1: WARNING: py:class reference target not found: DataFrame
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/sparse.py:docstring of hdmf.common.sparse.CSRMatrix:2: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/sparse.py:docstring of hdmf.common.sparse.CSRMatrix:4: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/sparse.py:docstring of hdmf.common.sparse.CSRMatrix:6: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/sparse.py:docstring of hdmf.common.sparse.CSRMatrix:8: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/table.py:docstring of hdmf.common.table.VectorData:16: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/table.py:docstring of hdmf.common.table.VectorIndex:9: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/table.py:docstring of hdmf.common.table.ElementIdentifiers:6: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/table.py:docstring of hdmf.common.table.DynamicTable:21: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/table.py:docstring of hdmf.common.table.DynamicTable:25: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/table.py:docstring of hdmf.common.table.docval.<locals>.dec.<locals>.func_call:14: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/table.py:docstring of hdmf.common.table.docval.<locals>.dec.<locals>.func_call:27: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/table.py:docstring of hdmf.common.table.docval.<locals>.dec.<locals>.func_call:29: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/table.py:docstring of hdmf.common.table.DynamicTableRegion:12: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/table.py:docstring of hdmf.common.table.EnumData:8: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/common/table.py:docstring of hdmf.common.table.EnumData:10: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/container.py:docstring of hdmf.container.Data:6: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/container.py:docstring of hdmf.container.docval.<locals>.dec.<locals>.func_call:7: WARNING: py:class reference target not found: function
/Users/rly/Documents/NWB/hdmf/src/hdmf/container.py:docstring of hdmf.container.DataRegion:4: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/container.py:docstring of hdmf.container.Table:27: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/data_utils.py:docstring of hdmf.data_utils.extend_data:1: WARNING: py:class reference target not found: np.ndarray
/Users/rly/Documents/NWB/hdmf/src/hdmf/data_utils.py:docstring of hdmf.data_utils.GenericDataChunkIterator._get_data:1: WARNING: py:class reference target not found: slices
/Users/rly/Documents/NWB/hdmf/src/hdmf/data_utils.py:docstring of hdmf.data_utils.GenericDataChunkIterator._get_data:1: WARNING: py:class reference target not found: np.ndarray
/Users/rly/Documents/NWB/hdmf/src/hdmf/data_utils.py:docstring of hdmf.data_utils.DataChunkIterator.__next__:5: WARNING: py:attr reference target not found: numpy.s_
/Users/rly/Documents/NWB/hdmf/src/hdmf/data_utils.py:docstring of hdmf.data_utils.assertEqualShape:1: WARNING: py:class reference target not found: np.ndarray
/Users/rly/Documents/NWB/hdmf/src/hdmf/data_utils.py:docstring of hdmf.data_utils.assertEqualShape:1: WARNING: py:class reference target not found: DataChunkIterator etc.
/Users/rly/Documents/NWB/hdmf/src/hdmf/data_utils.py:docstring of hdmf.data_utils.assertEqualShape:1: WARNING: py:class reference target not found: np.ndarray
/Users/rly/Documents/NWB/hdmf/src/hdmf/data_utils.py:docstring of hdmf.data_utils.assertEqualShape:1: WARNING: py:class reference target not found: DataChunkIterator etc.
/Users/rly/Documents/NWB/hdmf/src/hdmf/data_utils.py:docstring of hdmf.data_utils.assertEqualShape:1: WARNING: py:class reference target not found: ints
/Users/rly/Documents/NWB/hdmf/src/hdmf/data_utils.py:docstring of hdmf.data_utils.assertEqualShape:1: WARNING: py:class reference target not found: ints
/Users/rly/Documents/NWB/hdmf/src/hdmf/data_utils.py:docstring of hdmf.data_utils.DataIO:5: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/query.py:docstring of hdmf.query.HDMFDataset:2: WARNING: py:class reference target not found: zarr.core.Array
/Users/rly/Documents/NWB/hdmf/src/hdmf/spec/write.py:docstring of hdmf.spec.write.export_spec:1: WARNING: py:class reference target not found: ns_builder - NamespaceBuilder instance used to build
/Users/rly/Documents/NWB/hdmf/src/hdmf/spec/write.py:docstring of hdmf.spec.write.export_spec:1: WARNING: py:class reference target not found: new_data_types - Iterable
/Users/rly/Documents/NWB/hdmf/src/hdmf/spec/write.py:docstring of hdmf.spec.write.export_spec:1: WARNING: py:class reference target not found: specs that represent new data
/Users/rly/Documents/NWB/hdmf/src/hdmf/testing/testcase.py:docstring of hdmf.testing.testcase.TestCase:1: WARNING: py:class reference target not found: unittest.case.TestCase
/Users/rly/Documents/NWB/hdmf/src/hdmf/testing/testcase.py:docstring of hdmf.testing.testcase.TestCase.assertBuilderEqual:1: WARNING: py:class reference target not found: default=None
/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py:docstring of hdmf.utils.check_type:1: WARNING: py:class reference target not found: any
/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py:docstring of hdmf.utils.docval:38: WARNING: py:func reference target not found: dict
/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py:docstring of hdmf.utils.ExtenderMeta.post_init:3: WARNING: py:func reference target not found: type
/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py:docstring of hdmf.utils.get_data_shape:1: WARNING: py:class reference target not found: any object that support __len__
/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py:docstring of hdmf.utils.get_data_shape:1: WARNING: py:class reference target not found: .shape.
/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py:docstring of hdmf.utils.LabelledDict:47: WARNING: py:class reference target not found: function
/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py:docstring of hdmf.utils.LabelledDict:49: WARNING: py:class reference target not found: function
/Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py:docstring of hdmf.utils.StrDataset:1: WARNING: py:class reference target not found: h5py._hl.dataset.Dataset
/Users/rly/Documents/NWB/hdmf/src/hdmf/validate/validator.py:docstring of hdmf.validate.validator.SpecMatcher.spec_matches:3: WARNING: py:class reference target not found: Returns a list
/Users/rly/Documents/NWB/hdmf/src/hdmf/validate/validator.py:docstring of hdmf.validate.validator.SpecMatcher.spec_matches:3: WARNING: py:class reference target not found: tuples of
/Users/rly/Documents/NWB/hdmf/docs/source/overview_software_architecture.rst:84: WARNING: py:class reference target not found: hdmf.spec.AttributeSpec
/Users/rly/Documents/NWB/hdmf/docs/source/overview_software_architecture.rst:85: WARNING: py:class reference target not found: hdmf.spec.GroupSpec
/Users/rly/Documents/NWB/hdmf/docs/source/overview_software_architecture.rst:87: WARNING: py:class reference target not found: hdmf.spec.DatasetSpec
/Users/rly/Documents/NWB/hdmf/docs/source/overview_software_architecture.rst:89: WARNING: py:class reference target not found: hdmf.spec.LinkSpec
/Users/rly/Documents/NWB/hdmf/docs/source/overview_software_architecture.rst:93: WARNING: py:class reference target not found: hdmf.spec.DtypeSpec
/Users/rly/Documents/NWB/hdmf/docs/source/overview_software_architecture.rst:93: WARNING: py:class reference target not found: hdmf.spec.DatasetSpec
/Users/rly/Documents/NWB/hdmf/docs/source/tutorials/plot_external_resources.rst:199: WARNING: py:class reference target not found: hdmf.common.resources.Object.object_id
/Users/rly/Documents/NWB/hdmf/docs/source/tutorials/plot_external_resources.rst:199: WARNING: py:class reference target not found: hdmf.common.resources.Object.relative_path
/Users/rly/Documents/NWB/hdmf/docs/source/tutorials/plot_generic_data_chunk_tutorial.rst:196: WARNING: py:class reference target not found: hdmf.data_utils.DataChunk.selection
/Users/rly/Documents/NWB/hdmf/docs/source/tutorials/plot_term_set.rst:166: WARNING: py:func reference target not found: hdmf.term_set.TermSet:view_set
/Users/rly/Documents/NWB/hdmf/docs/source/validation.rst:6: WARNING: py:mod reference target not found: pynwb

I see the h5py issues like /Users/rly/Documents/NWB/hdmf/src/hdmf/utils.py:docstring of hdmf.utils.StrDataset:1: WARNING: py:class reference target not found: h5py._hl.dataset.Dataset. That is from StrDataset extending h5py.Dataset. I'm not sure why that is not being resolved automatically.

@stephprince i know it's tedious, but could you look into some of the other issues?

I see zarr warnings. Can you add Zarr to the intersphinx mapping? That might get rid of those warnings.

Can you change "np.ndarray" in docstrings to "numpy.ndarray"? That should resolve those.

For h5py.RegionReference, it looks like RegionReference has no high-level h5py documentation. We can just do a hardcoded replacement of that with RegionReference as code instead of :py:class:~h5py.RegionReference``. Same with Intracomm.

hdmf.spec.AttributeSpec should be hdmf.spec.spec.AttributeSpec I think, because AttributeSpec is defined in the module hdmf.spec.spec

In tutorials, for cases where it is not possible to link to documentation, such as hdmf.data_utils.DataChunk.selection, we can just use code syntax instead, e.g., DataChunk.selection (with backticks around it)

That might get us down to <20 warnings, which is more easily scannable when we look for broken docs.

@rly
Copy link
Contributor

rly commented Jan 17, 2024

Can you also update the sphinx linkcheck workflow to use this flag?

@stephprince
Copy link
Contributor Author

stephprince commented Jan 17, 2024

yes I can take a look at the other issues and update the workflow for this flag. I had just focused on the ones that impacted pynwb, but it looks like there are very similar ones in hdmf as well.

For the WARNING: py:class reference target not found: h5py._hl.dataset.Dataset one, from my understanding of the objects.inv file that sphinx uses, the h5py references weren't resolving because they should be referred to as h5py.Dataset. You can look specifically at the h5py mapping by running: python -m sphinx.ext.intersphinx https://docs.h5py.org/en/latest/objects.inv > objects.txt.

Edit: I misunderstood the StrDataset example you were highlighting. I'm also not sure why that's not getting resolved but can look into it

@rly
Copy link
Contributor

rly commented Jan 17, 2024

I misunderstood the StrDataset example you were highlighting. I'm also not sure why that's not getting resolved but can look into it

I'd say look into it briefly, but no worries if you can't figure it out. There is a lot of sphinx magic here and It's OK if there are some that we cannot figure out.

@stephprince
Copy link
Contributor Author

It should be down to 9 warnings with these fixes. With the Intracomm and RegionReference, even changing them to strings or lists of strings doesn't seem to fix the reference warnings, so I left them as types for now.

Instead of changing the flags in the workflows, I added nitpick=True to the conf.py file so that it's consistent across all the make/workflow files. We could also add some of these errors to a nitpick_ignore section in the conf.py file if we wanted.

@oruebel
Copy link
Contributor

oruebel commented Jan 18, 2024

Thanks for all these fixes. It's amazing how many bad links creep in without automatic link checking.

src/hdmf/spec/spec.py Outdated Show resolved Hide resolved
Co-authored-by: Ryan Ly <rly@lbl.gov>
rly
rly previously approved these changes Jan 18, 2024
@rly
Copy link
Contributor

rly commented Jan 18, 2024

Looks good to me. Thanks for all these fixes.

@rly
Copy link
Contributor

rly commented Jan 18, 2024

@stephprince could you add the remaining warnings to nitpick_ignore? It would be nice to have that clean and raise nitpick warnings as errors.

@stephprince
Copy link
Contributor Author

Yes I can do that. It looks like the BaseStorageSpec one also impacts pynwb so I can add it to that nitpick_ignore as well

@rly
Copy link
Contributor

rly commented Jan 18, 2024

Thanks. I don't see BaseStorageSpec triggering a warning in the latest github action. Did it magically go away?

@stephprince
Copy link
Contributor Author

It looks like it only gave a reference warning on the pynwb side of things when pynwb.spec.NWBAttributeSpec has AttributeSpec as its base class.

Copy link
Contributor

@rly rly left a comment

Choose a reason for hiding this comment

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

Great!

@stephprince stephprince merged commit 3a3dd59 into dev Jan 19, 2024
28 checks passed
@stephprince stephprince deleted the internal-doc-links branch January 19, 2024 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants