DOC: Enabling nitpicky docs build #416
Conversation
|
I've run into a few API references again that are not linkified properly, so I suppose this should be picked up reasonably soon to avoid further rot in the documentation. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #416 +/- ##
=======================================
Coverage 80.32% 80.32%
=======================================
Files 52 52
Lines 6176 6176
=======================================
Hits 4961 4961
Misses 1215 1215 ☔ View full report in Codecov by Sentry. |
bsipocz
force-pushed
the
DOC_nitpicky
branch
2 times, most recently
from
41f0591 to
4f67293
Compare
|
This should be ready to go. |
| "Datamodel", "Ivoid", "UCD", "Spatial", "Spectral", "Temporal", | ||
| "Constraint", "build_regtap_query", "RegTAPFeatureMissing"] | ||
| # Classes from this module are exposed at the higher level namespace, not listing them here | ||
| __all__ = ["build_regtap_query"] |
There was a problem hiding this comment.
I'm slightly concerned about client code whose imports may rely on the old access, but I'm guessing these constraint classes are unlikely to be used directly in much client code, so my concern is pretty small.
There was a problem hiding this comment.
I think this will only affect those that import * from rtcons which is an anti-pattern. The module's __init__ specifies individual classes but it doesn't include build_regtap_query whether on purpose or not.
There was a problem hiding this comment.
Yes exactly. Direct imports will keep working, this is just to avoid having a class exposed at two API locations, and then having multiple API reference entry points.
There was a problem hiding this comment.
IMO, there are a lot of places where the API could be improved or fixed, or at least consolidated where to expose a class, but this PR only goas as far to fix the inconsistencies that came up through the documentation.
There was a problem hiding this comment.
Wow, thanks @bsipocz for all these fixes and cleanup!! This should ensure better docs for a while.
| This module contains submodules which help handle auth when | ||
| communicating with virtual observatory services. | ||
|
|
||
| Reference/API | ||
| ============= | ||
|
|
||
| .. automodapi:: pyvo.auth | ||
| :no-inheritance-diagram: |
There was a problem hiding this comment.
Not sure what this does but I trust you've added it with a purpose.
There was a problem hiding this comment.
Not sure why I did it, commit is back from Thu Jan 26 2023 19:03:58 :)
There was a problem hiding this comment.
Ahh, ok, probably I though that this one is not super helpful, there is not much inheritance going on:
https://pyvo.readthedocs.io/en/latest/auth/index.html#class-inheritance-diagram
|
Given the approvals, I go ahead and merge this, and then see whether CI picks up something new (building docs locally is always challenging on OSX as it's case insensitive and we run into that issues a couple of times) |
There was a problem hiding this comment.
E.g. look at this file for a subset of the API problems. We keep referencing classes that are not public API exposed. Some of them should not be used, but some may need to be added into the public API nevertheless, e.g. exceptions.
|
On Fri, Feb 16, 2024 at 01:37:48PM -0800, Adrian wrote:
I think this will only affect those that import `*` from `rtcons`
which is an anti-pattern. The module's `__init__` specifies
I agree we do not want to enable import *-ing.
individual classes but it doesn't include `build_regtap_query`
whether on purpose or not.
That was not on purpose. I certainly consider build_regtap_query a
first class API function.
|
DOC: Enabling nitpicky docs build
DOC: Enabling nitpicky docs build
This should show the warnings we get during building the docs and closes #411.
The PR is a draft as I won't be able to build the docs locally due to a very old upstream bug (sphinx-doc/sphinx#1495), and reached a point where it would be useful to see how much more warnings are still there.