Fix api doc generation for aliases #2063
Conversation
Aliases were previously resulting in the doc generation for the main object to be skipped.
doc/generate_module_rst.py
Outdated
| if id(f.obj) not in seen: | ||
| seen.add(id(f.obj)) | ||
| # See comment above (for classes) about aliases. | ||
| if f.name.endswith("." + f.obj.__name__): |
There was a problem hiding this comment.
(strictly speaking we could also have an alias for one of the dispatcher functions handled above, though in practice I expect we won't)
There was a problem hiding this comment.
Good spot; I missed that. I'll add that as well for completeness.
doc/generate_module_rst.py
Outdated
| for impl in impls: | ||
| seen.add(id(impl)) | ||
| # See comment below (for classes) about aliases. | ||
| if function.name.endswith("." + function.obj.__name__): |
There was a problem hiding this comment.
Shouldn't this be checking for aliases of the implementations impl rather than of the dispatcher function?
There was a problem hiding this comment.
I guess you could have aliases at both levels, dispatcher and implementations. Adding a check at the implementation level is more tricky since there is no existing Documentable* wrapper class for the implementations -> so there is no pathname to reference, only the function objects themselves. It seems to me that it is unlikely we will have aliases for implementations, since there is not a need for users to call them directly. What do you think?
doc/generate_module_rst.py
Outdated
| for f in module.functions: | ||
| if id(f.obj) not in seen: | ||
| seen.add(id(f.obj)) | ||
| # See comment above (for classes) about aliases. |
There was a problem hiding this comment.
Thinking about this a bit more, wouldn't it make more sense to just use a different key? That way we would also remove duplicate aliases.
| # See comment above (for classes) about aliases. | |
| key = (id(f.obj), f.name[f.name.rfind(".")+1:]) | |
| if key not in seen: | |
| seen.add(key) | |
| new_functions.append(f) |
There was a problem hiding this comment.
That makes sense. I have updated the keys.
This required changes to the dispatcher part as well to use the function implementation __name__ in the key, so that it then later matches correctly here.
There was a problem hiding this comment.
This addresses your previous comment as well I think. The check is now against the implementation instead of the dispatcher itself.
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## develop #2063 +/- ##
========================================
Coverage 98.06% 98.06%
========================================
Files 97 97
Lines 5436 5436
========================================
Hits 5331 5331
Misses 105 105 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
PR type: doc improvement
Related issue(s)/PRs: resolves #2056
Summary
Proposed changes
SquaredExponentialandConstantkernels.RBFandBiasaliases (respectively) were present.Doc example with missing classes: https://gpflow.github.io/GPflow/2.7.1/api/gpflow/kernels/index.html#gpflow-kernels-rbf
Generated fixed documentation locally and checked the classes are present.
Fully backwards compatible: yes
PR checklist
make format)make check-all)