Fix cross reference #3273

ericpre · 2023-11-24T10:51:42Z

This PR fixes many links to the API references. Enable sphinx "nitpicky" to check that there is no broken reference.
With the API changes, it is important that the reference are working, as it helps identifying if something is missing from the API references, beside being convenient to navigate.

Progress of the PR

Fix sphinx cross references to API reference,
- [x] use numpydoc
- [x] enable sphinx "nitpicky" to check any broken reference.
update docstring (if appropriate),
update user guide (if appropriate),
Remove mutable argument used as default value,
Set some model component attributes as property
add an changelog entry in the upcoming_changes folder (see upcoming_changes/README.rst),
Check formatting changelog entry in the readthedocs doc build of this PR (link in github checks)
[n/a] add tests,
ready for review.

hakonanes · 2023-11-24T11:49:29Z

Great work, it is really a huge task to get Sphinx and friends to automatically document the complete API nicely (including just what you want).

I was wondering. Have you built the API by just listing the modules in a top index.rst and letting Sphinx decide which API members are listed using the __all__ lists in module/__init__.py and Sphinx templating? Such as done in kikuchipy (index.rst, overwritten templates)? Of course, HyperSpy's API is larger...

Also, have you tried controlling which members are listed by autodoc by overwriting the autodoc-skip-member function? I have a PR open in orix where I try to exclude all members defined outside orix: https://github.com/pyxem/orix/blob/ae619f40304bf0faf3cc535f32fc1a6362f4c736/doc/conf.py#L345-L364.

ericpre · 2023-11-24T14:46:56Z

I did look at what you mentioned, and I found it that it doesn't play well with how hyperspy is currently structured. This is more simple after the split but still came to the conclusion that defining what API modules goes where explicitly is more straightforward and simple rather than using templates that needs to be adapted to hyperspy's need to generate the API automatically. The fact that the documentation is succinct and that it is difficult (and slow) to troubleshoot when something doesn't work, did put me off quite a bit.
At the end of the day, it is not that much to define explicitly and the maintenance will be straightforward, so I didn't think it was beneficial. I find the current API reference (v1.7) very impractical and confusing and I hardly use it... The issue is not what is included but how it is structured.

hakonanes · 2023-11-24T15:22:19Z

I see, thank you for clarifying. I'm quite sure I'll learn some nice tricks from this PR: feel free to ping me for a review.

maintenance will be straightforward

Having a curated API reference, as you introduce here, rather than one automatically generated (as in kikuchipy/orix) can be straightforward to maintain, if the API doesn't change too quickly, and one remembers to add new members (classes, methods, functions, etc.). We had a curated API for orix, but it quickly became outdated as a constant stream of new features weren't continuously added... Just something to be aware of.

ericpre · 2023-11-24T17:17:20Z

The important point of this PR is that the doc build will fail if a cross reference is broken. This is important, because when a new API is added, it is documented and it comes cross references to the relevant API - usually we do this fairly well. This should catch anything new and missing from the API reference.

I would expect that it is rare (when restructuring a module, for example) that we need to update the API reference manually, because:

new method of existing class are added automatically with autoclass/automodule
new classes are very rare and they are covered by automodule (except for some cases)

codecov · 2023-11-25T09:00:28Z

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (224dca7) 80.86% compared to head (f017fac) 80.84%.
Report is 39 commits behind head on RELEASE_next_major.

Files	Patch %	Lines
hyperspy/model.py	83.33%	5 Missing and 1 partial ⚠️
hyperspy/learn/mva.py	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                  Coverage Diff                   @@
##           RELEASE_next_major    #3273      +/-   ##
======================================================
- Coverage               80.86%   80.84%   -0.02%     
======================================================
  Files                     140      140              
  Lines                   20399    20475      +76     
  Branches                 4825     4838      +13     
======================================================
+ Hits                    16495    16553      +58     
- Misses                   2829     2859      +30     
+ Partials                 1075     1063      -12

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

hakonanes · 2023-11-27T08:07:56Z

I agree that as long as contents of modules (functions and classes with attributes and methods) are listed automatically the maintenance burden should be reasonable.

ericpre · 2023-11-29T21:02:44Z

@hakonanes, @jlaehne, do you want to review this PR?

…ers` as `Component` property

hakonanes · 2023-11-30T17:43:17Z

I'm sorry, but I don't have time until Monday...

hakonanes

Stupendous amount of work this must have been, I'm impressed!

I've gone through 131/142 files here on GitHub. I've just briefly checked the API reference in the docs built from this PR. Plan to look through the remaining files tomorrow or Sunday.

No need to commit my suggested changes, feel free to do the changes yourself.

hakonanes · 2023-12-01T21:45:54Z

doc/conf.py

+    "module",
+}
+
+# if Version(numpydoc.__version__) >= Version("1.6.0rc0"):


Will this be useful later on? I see version 1.6 of numpydoc was released in August (here).

Yes, it could be, to check the formatting of the docstring, but I think that this is not a priority. I don't know how much editing is required in the current state but better in a separate PR.

doc/reference/base_classes/signal/index.rst

doc/reference/metadata.rst

doc/user_guide/io.rst

doc/user_guide/model/fitting_model.rst

hyperspy/learn/ornmf.py

hyperspy/learn/rpca.py

hyperspy/learn/svd_pca.py

hyperspy/misc/math_tools.py

upcoming_changes/3273.bugfix.rst

ericpre · 2023-12-07T12:26:54Z

@hakonanes, please let me know when you finish your review.

hakonanes · 2023-12-07T21:30:33Z

I'll finish tomorrow evening, sorry for the delay.

hakonanes

Here're the rest of my comments. Again, good work!

hyperspy/component.py

hyperspy/learn/mva.py

hyperspy/model.py

hyperspy/models/model1d.py

hyperspy/models/model2d.py

hyperspy/roi.py

… not needed

ericpre

Thank you @hakonanes, I fixed the typos and the links.

@jlaehne, do you want to review this PR?

hyperspy/learn/mva.py

ericpre · 2023-12-09T11:57:05Z

doc/conf.py

+    "module",
+}
+
+# if Version(numpydoc.__version__) >= Version("1.6.0rc0"):


Yes, it could be, to check the formatting of the docstring, but I think that this is not a priority. I don't know how much editing is required in the current state but better in a separate PR.

hyperspy/model.py

hyperspy/models/model1d.py

hyperspy/model.py

jlaehne · 2023-12-11T07:43:35Z

I'll try to browse over it these days, but I don't think I'll manage an as detailed review as @hakonanes

doc/user_guide/io.rst

doc/user_guide/model/indexing_model.rst

hyperspy/_signals/complex_signal2d.py

hyperspy/_signals/signal1d.py

hyperspy/learn/ornmf.py

hyperspy/learn/rpca.py

hyperspy/model.py

ericpre added affects: documentation type: bug-fix labels Nov 24, 2023

ericpre added this to the v2.0 Split milestone Nov 24, 2023

ericpre force-pushed the docstring_improvement branch from 6be9f89 to add17e0 Compare November 25, 2023 09:19

ericpre force-pushed the docstring_improvement branch from add17e0 to 0df49ce Compare November 29, 2023 20:47

ericpre requested a review from jlaehne November 29, 2023 21:01

ericpre added 17 commits November 30, 2023 18:07

Improve docstring Signal1D

f00637b

Fix model docstring

30451c3

Fix release notes formatting

50db111

Fix formating docstrings signals

59219aa

Fix docstring model and utilities

8f67499

Enable numpydoc and fix section headings in docstrings

3be349a

Fix docstrings model

37304c6

Add machine learning in reference API

00bb97f

Fix model API reference

4670fcb

Enable nitpicky to check API links

31a7e38

Fix API links in user guide: visualisation.rst and signal2d.rst

d1fa63b

Fix API links in user guide: model add free_parameters and `paramet…

a9ee5bd

…ers` as `Component` property

Fix xref to signals

3792bcd

Remove EELSDB section

54ed61c

Fix xref

ee79efc

Fix xref

50d04e6

Fix xref samfire

e56a4b7

ericpre added 8 commits November 30, 2023 18:07

Fix xref model, components

f7ed969

Fix xref machine learning

6d976b6

Fix xref axes

f715b66

Fix xref signals

c5beb79

Fix xref plot, samfire and fix mutable argument as default

5c1b83b

Fix xref components, data modules

a307495

Fix xref release notes

49de0c3

Update dev guide

ce8faac

ericpre force-pushed the docstring_improvement branch from 0df49ce to 5281d51 Compare November 30, 2023 17:21

ericpre added the status: needs review label Nov 30, 2023

hakonanes reviewed Dec 1, 2023

View reviewed changes

hakonanes reviewed Dec 8, 2023

View reviewed changes

ericpre added 3 commits December 9, 2023 18:14

Remove all :py as py is the default in sphinx and it is therefore…

3d2eeee

… not needed

Add changelog entry

d46a011

Fix typos and sphinx links

93a1719

ericpre force-pushed the docstring_improvement branch from 5281d51 to 93a1719 Compare December 9, 2023 18:23

ericpre commented Dec 9, 2023

View reviewed changes

hakonanes approved these changes Dec 10, 2023

View reviewed changes

hyperspy/models/model1d.py Outdated Show resolved Hide resolved

hyperspy/model.py Show resolved Hide resolved

jlaehne mentioned this pull request Dec 11, 2023

Consolidate package metadata #3268

Merged

9 tasks

jlaehne reviewed Dec 11, 2023

View reviewed changes

Fix typos and sphinx links, take 2

f017fac

jlaehne approved these changes Dec 12, 2023

View reviewed changes

jlaehne removed the status: needs review label Dec 12, 2023

jlaehne merged commit 2ec4ba8 into hyperspy:RELEASE_next_major Dec 12, 2023
23 of 24 checks passed

ericpre deleted the docstring_improvement branch December 14, 2023 19:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix cross reference #3273

Fix cross reference #3273

ericpre commented Nov 24, 2023 •

edited

hakonanes commented Nov 24, 2023

ericpre commented Nov 24, 2023

hakonanes commented Nov 24, 2023

ericpre commented Nov 24, 2023

codecov bot commented Nov 25, 2023 •

edited

hakonanes commented Nov 27, 2023

ericpre commented Nov 29, 2023

hakonanes commented Nov 30, 2023

hakonanes left a comment

hakonanes Dec 1, 2023

ericpre Dec 9, 2023

ericpre commented Dec 7, 2023

hakonanes commented Dec 7, 2023

hakonanes left a comment •

edited

ericpre left a comment

ericpre Dec 9, 2023

jlaehne commented Dec 11, 2023

Fix cross reference #3273

Fix cross reference #3273

Conversation

ericpre commented Nov 24, 2023 • edited

Progress of the PR

hakonanes commented Nov 24, 2023

ericpre commented Nov 24, 2023

hakonanes commented Nov 24, 2023

ericpre commented Nov 24, 2023

codecov bot commented Nov 25, 2023 • edited

Codecov Report

hakonanes commented Nov 27, 2023

ericpre commented Nov 29, 2023

hakonanes commented Nov 30, 2023

hakonanes left a comment

Choose a reason for hiding this comment

hakonanes Dec 1, 2023

Choose a reason for hiding this comment

ericpre Dec 9, 2023

Choose a reason for hiding this comment

ericpre commented Dec 7, 2023

hakonanes commented Dec 7, 2023

hakonanes left a comment • edited

Choose a reason for hiding this comment

ericpre left a comment

Choose a reason for hiding this comment

ericpre Dec 9, 2023

Choose a reason for hiding this comment

jlaehne commented Dec 11, 2023

ericpre commented Nov 24, 2023 •

edited

codecov bot commented Nov 25, 2023 •

edited

hakonanes left a comment •

edited