Skip to content

Conversation

Andy-Grigg
Copy link
Contributor

@Andy-Grigg Andy-Grigg commented Aug 22, 2024

Closes #650

This PR solves the problem of 'leaking' ansys.openapi.common documentation specifics into packages that subclass the types in this package.

Broken xrefs

Missing xrefs in numpydoc docstrings was fixed by fully qualifying type references in docstrings for all commonly inherited classes and add a note in the contributing guide.

Inconsistent versionadded directives

This is achieved by wrapping .. versionadded:: ReStructuredText directives in a pair of .. only:: blocks that serve to switch the documentation behavior depending on whether a tag is present or not. If the tag is present, it is assumed that we are building 'standalone' docs, i.e. docs for this package, and the versionadded directive is included. If the tag is not present, it is assumed we are building docs for some other package that inherit docstrings from this package. In this case, we use an .. info:: directive and include a link to the openapi docs for context.

Also requires a sphinx conf.py changes to add the tag automatically during the build.

@Andy-Grigg Andy-Grigg requested a review from da1910 August 22, 2024 17:59
@github-actions github-actions bot added bug Something isn't working documentation Improvements or additions to documentation labels Aug 22, 2024
@Andy-Grigg
Copy link
Contributor Author

Andy-Grigg commented Aug 22, 2024

Example of the versionadded code in ansys-openapi-common docs:

image

Example in recordlists with a clickable reference:

image

Copy link

codecov bot commented Aug 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.44%. Comparing base (76f2694) to head (1a88fde).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #651   +/-   ##
=======================================
  Coverage   94.44%   94.44%           
=======================================
  Files           7        7           
  Lines         792      792           
=======================================
  Hits          748      748           
  Misses         44       44           

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

@Andy-Grigg Andy-Grigg changed the title Ensure directives included in docstrings support subclassing by other packages Allow packages to build documentation if they inherit classes in this package Aug 22, 2024
Copy link
Collaborator

@da1910 da1910 left a comment

Choose a reason for hiding this comment

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

This is a bit of a shame, how hard is it to make a custom directive maybe? I mean if it works then it's fine, but it's a shame to have to have the same docstring twice

Co-authored-by: Doug Addy <da1910@users.noreply.github.com>
@Andy-Grigg
Copy link
Contributor Author

This is a bit of a shame, how hard is it to make a custom directive maybe? I mean if it works then it's fine, but it's a shame to have to have the same docstring twice

Interesting idea. I'll have a look and see if it will work. It would probably need to accept the version number, the 'external' docstring, and probably the tag that's used to determine if it is internal or external, but that could also be hardcoded I suppose.

I'll look into it and see how it goes.

@Andy-Grigg
Copy link
Contributor Author

Actually, I'm not sure a custom directive would be all that useful, because inheriting packages would also need to have that directive defined to understand it. Otherwise sphinx would probably just crash.

@da1910
Copy link
Collaborator

da1910 commented Aug 22, 2024

Ah yeah I guess it would need to live in the Ansys sphinx theme, and it's quite specific I think to our setup

@Andy-Grigg
Copy link
Contributor Author

Exactly. Plus, if someone inherited from this package and chose not to use that theme, they'd also get a failure. We could also pull the directive out into a separate package, but again it's then putting some onus on the inheriting package to do something extra to be able to use this package.

At least with this current approach, this package takes care of everything, even if it does add a bit of overhead.

@Andy-Grigg Andy-Grigg requested a review from da1910 August 23, 2024 14:12
@Andy-Grigg Andy-Grigg enabled auto-merge August 23, 2024 14:18
@Andy-Grigg Andy-Grigg added this pull request to the merge queue Aug 23, 2024
Merged via the queue into main with commit d89dccd Aug 23, 2024
Andy-Grigg added a commit that referenced this pull request Aug 26, 2024
… package (#651)

Co-authored-by: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com>
Co-authored-by: Doug Addy <da1910@users.noreply.github.com>
@Andy-Grigg Andy-Grigg deleted the fix/clarify-versionadded-directive branch August 26, 2024 13:59
@Andy-Grigg Andy-Grigg mentioned this pull request Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

with_credentials docstring breaks docs builds

3 participants