chore: resolve (or suppress) all Sphinx warnings #14349
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## edge #14349 +/- ##
=======================================
Coverage 68.11% 68.11%
=======================================
Files 2502 2502
Lines 71580 71580
Branches 9111 9111
=======================================
+ Hits 48755 48756 +1
Misses 20696 20696
+ Partials 2129 2128 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Yay, thank you!
RIP lightbulb.jpg.
Is the warning suppression strategy valid? Are there things that should be cleaned up in source instead? Should we publicize and document some of these instead?
Yeah, this makes sense.
I haven't looked very closely at the list of stuff that we need to ignore because I'm not very API-brained at the moment (sorry). I think it's likely that a lot of them are pointing to things that we should resolve "properly," like you said. But it's definitely a good idea to ignore them for now so we can treat new warnings as errors and stop things from getting worse.
api/docs/v2/deck_slots.rst
Outdated
| The :py:class:`TrashBin` class doesn't have any callable methods, so you don't have to save the result of ``load_trash_bin()`` to a variable, especially if your protocol only loads a single trash container. Being able to reference the trash bin by name is useful when dealing with multiple trash containers. | ||
| The ``TrashBin`` class doesn't have any callable methods, so you don't have to save the result of ``load_trash_bin()`` to a variable, especially if your protocol only loads a single trash container. Being able to reference the trash bin by name is useful when dealing with multiple trash containers. |
There was a problem hiding this comment.
Let's publicize opentrons.protocol_api.TrashBin and opentrons.protocol_api.WasteChute, I think. Doesn't have to happen in this PR though.
There was a problem hiding this comment.
I think I'll do it here, since otherwise I'll have to refer back to this PR to figure out which occurrences I want to re-link.
api/docs/v2/conf.py
Outdated
| # The goal here is to pass through any warnings for bad targets of MANUALLY | ||
| # created links. | ||
| nitpick_ignore_regex = [ | ||
| ("py:class", r".*\._.*"), # anything private |
There was a problem hiding this comment.
We generally do want it to be an error if we accidentally mention something private in the docs. It looks like this is only triggering on TrashBin and WasteChute now, which makes sense. Once we publicize those, I hope we can delete this rule.
There was a problem hiding this comment.
Good point. I thought this was catching something else too, but it must be addressed by one of the less broad filters.
I did have to add types.DeckSlotName in its place, because just including that in the reference was a can of worms, which is now RTC-386.
| If the labware is on a module, a :py:class:`ModuleContext`. | ||
| If the labware is on a module, a module context. |
There was a problem hiding this comment.
We don't have to list them all out, but it might be worth linking some example of a module context here and in the other places where we're replacing broken ModuleContext links. Again, doesn't have to happen in this PR.
There was a problem hiding this comment.
A couple other options here:
- Link to the top of the Modules section of the reference (which starts with Heater-Shaker because it's first alphabetically). We could even insert some text into
new_protocol_api.rstto explain that everything in that section is a "module context". - Link to the Module Setup section, which talks about module contexts, although somewhat obliquely.
ecormany
force-pushed
the
destroy-all-sphinx-warnings
branch
from
fc84532
to
0fb3ba7
Compare
Overview
Every Python docs build was spewing hundreds of Sphinx warnings. This prevented us from treating warnings as errors in CI. It also made it really hard to tell if you introduced some new problem, like a broken link.
This PR brings the current docs to zero reported warnings. Warnings with easily fixable root causes are actually fixed. Warnings resulting from type hints in the API Reference have been suppressed. Is this a kludge? Kinda. But the only other way out — as far as I can tell — is to actually publicly document every single thing in those type hints. I believe we have multiple reasons not to do that. Hopefully each warning suppression pattern that I've added to
conf.pymatches all and only a category of things we have motivated reasons to not document.Addresses RTC-198 (except for hardware docs).
Test Plan
Changelog
Fix warnings
TrashBinandWasteChute.ModuleContextto refer toThermocyclerContext,HeaterShakerContext, etc.pop()(what are the Python docs doing??)Suppress warnings
Adds a long list of
nitpick_ignore_regexoptions.CI
v%builds away from allowing warnings. (v1already had no warnings, hooray.)Review requests
Is the warning suppression strategy valid? Are there things that should be cleaned up in source instead? Should we publicize and document some of these instead?
Risk assessment
Low. Changes CI slightly but should only fail for actual Sphinx problems that we should catch as soon as they happen.