Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update and reorganise sphinx nitpick_ignore list #2271

Merged
merged 3 commits into from
Feb 5, 2022

Conversation

dhomeier
Copy link
Collaborator

@dhomeier dhomeier commented Feb 4, 2022

Description

Following up on #2237 (comment) this is adding a PyQt5.sip entry which raised a warning locally, although it did not seem to affect the CI.
Also cleaned up the entire list a bit, including removing all generic and internal object exceptions – hopefully all further warnings in our own docs can be fixed rather than ignored.
Apart from all the external Qt* references, what remains at this point are some BasicToolbar methods that are inherited from PyQt5.QtWidgets.QToolBar, and similar ones from matplotlib in glue.viewers; and thus also really outside of our own docs.
The module warnings for organization.rst appear to be due to no API documentation existing at that level, so why it would be nice to have actual references there, explicitly disabling the links also silences them.

As a note, as of Sphinx 4.1 (seems we have 4.4 everywhere) the entire list of external ignores could also be replaced by a more concise

nitpick_ignore_regex = [('py:class', r'PyQt5\.QtCore\.Q[A-Z][a-zA-Z]+'),
                        ('py:class', r'PyQt5\.QtWidgets\.Q[A-Z][a-zA-Z]+'),
                        ('py:class', r'Qt\.[A-Z][a-zA-Z]+'),
                        ('py:class', r'QPalette\.[A-Z][a-zA-Z]+'),
                        ('py:class', r'QWidget\.[A-Z][a-zA-Z]+'),
                        ('py:class', r'Q[A-Z][a-zA-Z]+')]

@dhomeier dhomeier requested a review from astrofrog February 4, 2022 18:55
@codecov
Copy link

codecov bot commented Feb 4, 2022

Codecov Report

Merging #2271 (8fe3006) into main (5604178) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2271   +/-   ##
=======================================
  Coverage   88.07%   88.07%           
=======================================
  Files         247      247           
  Lines       23279    23279           
=======================================
  Hits        20502    20502           
  Misses       2777     2777           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 164424f...8fe3006. Read the comment docs.

Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

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

Looks good! Feel free to merge once the CI passes.

I'd also be fine with requiring Sphinx 4.1+ if you want to use the regexps for the nitpicks, so I'll leave that up to you (could be done in a follow-up PR).

@dhomeier
Copy link
Collaborator Author

dhomeier commented Feb 5, 2022

I'd also be fine with requiring Sphinx 4.1+ if you want to use the regexps for the nitpicks, so I'll leave that up to you (could be done in a follow-up PR).

Probably don't need to make it a strict requirement – since older versions will just ignore the unknown config field, if someone wants to build with pre-4.1 they will merely see the bunch of warnings again.

@astrofrog
Copy link
Member

Sounds good, feel free to merge once ready!

@dhomeier
Copy link
Collaborator Author

dhomeier commented Feb 5, 2022

Thanks for reviewing!

@dhomeier dhomeier merged commit 3d09f59 into glue-viz:main Feb 5, 2022
@dhomeier dhomeier deleted the sphinx-nitpick-ignores branch February 5, 2022 13:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants