radical-cybertools / radical.pilot

RADICAL-Pilot
http://radical-cybertools.github.io/radical-pilot/index.html
Other
54 stars 23 forks source link

make sure that `radical.pilot.utils` as documented also #2860

Closed andre-merzky closed 1 year ago

andre-merzky commented 1 year ago

This fixes #2706

codecov[bot] commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 40.18%. Comparing base (800076f) to head (de74377). Report is 3352 commits behind head on devel.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## devel #2860 +/- ## ======================================= Coverage 40.18% 40.18% ======================================= Files 94 94 Lines 10262 10262 ======================================= Hits 4124 4124 Misses 6138 6138 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

eirrgang commented 1 year ago

Do we have a well documented doc string for every member of each module?

Module level docstrings are missing for most package modules (__init__.py files). To support intersphinx links to package modules like radical.pilot and radical.pilot.states, I sprinkled rst directives like .. py:module:: radical.pilot into the docs on previous occasions. There is currently no docstring or target for radical.pilot.utils, though.

Without higher-level docstrings, auto-extracted member docstrings often get skipped. (This behavior seems inconsistent, and may vary across sphinx versions or in terms of config settings that aren't intuitively coupled.)

mturilli commented 1 year ago

Thanks @eirrgang . You clearly know more about Sphinx than I do. Is this PR useful as it is? How would you suggest to proceed?

eirrgang commented 1 year ago

Thanks @eirrgang . You clearly know more about Sphinx than I do. Is this PR useful as it is? How would you suggest to proceed?

I think it would be good to make sure that radical.pilot.utils is defined in the doctree so that :py:mod: directives will work.

Option 1

Add a module level docstring to radical/pilot/utils/__init__.py and a .. automodule:: radical.pilot.utils directive in the rst file.

Option 2

Insert .. py:module:: radical.pilot.utils below the "Utilities and helpers" heading (before the automodule directives for modules that do have module-level docstrings) to create the target without any additional text.

andre-merzky commented 1 year ago

I would suggest to keep this PR about the inclusion of the submodule and to open a new ticket about the missing doc strings. Is that ok for y'all?

eirrgang commented 1 year ago

I would suggest to keep this PR about the inclusion of the submodule and to open a new ticket about the missing doc strings. Is that ok for y'all?

That seems fine. I suppose the current issue is "publish the docs". The follow-up issue(s) could be "let the following sphinx directive link to something":

:py:mod:`radical.pilot.utils`

and, if necessary, "let .. auto***:: ... ... :members: find documented members of entities that do not (currently) have docstrings"

andre-merzky commented 1 year ago

I would suggest to keep this PR about the inclusion of the submodule and to open a new ticket about the missing doc strings.

That ticket is open now (#2877).