Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix trait_documenter for properties #1246

Merged
merged 2 commits into from
Jul 21, 2020

Conversation

mdickinson
Copy link
Member

@mdickinson mdickinson commented Jul 18, 2020

This PR fixes the trait_documenter Sphinx extension so that it no longer fails on regular Python properties. It does this by modifying the can_document_member function so that it doesn't recognise generic traits. This makes sense, since a generic trait is deliberately designed to behave like a standard Python object attribute.

Mostly fixes #1238, but independently of this fix, it would be good to fix the trait-documenter to be less fragile, so that if it fails in some other situation in the future then that failure doesn't prevent the Sphinx build from completing.

EDIT: See #1247 for the "make trait-documenter less fragile" fix.

Copy link
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

LGTM

@rahulporuri
Copy link
Contributor

Two comments for the record that are implicit but will be good to document explicitly :

  • Ignoring the python properties in the TraitDocumenter doesn't mean that they don't get documented at all. They simply get passed on to other sphinx extensions e.g. apidoc which document them.
  • Ignoring all generic_trait types in a class name as a proxy for python properties can be considered overkill but in practice, there are few, if no, examples where users explicitly use generic_trait. So this assumption makes sense for now - until someone who heavily uses generic_trait complains.

@mdickinson
Copy link
Member Author

Thanks, @rahulporuri. I'll merge this into master, then resolve the conflicts with #1247.

@mdickinson mdickinson merged commit 464ec60 into master Jul 21, 2020
@mdickinson mdickinson deleted the fix/trait-documenter-for-properties branch July 21, 2020 12:36
mdickinson added a commit that referenced this pull request Jul 21, 2020
* Add a failing tests

* And fix the failing test

(cherry picked from commit 464ec60)
mdickinson added a commit that referenced this pull request Jul 21, 2020
* Expand user manual to mention dispatch (#1195)

(cherry picked from commit 64c8c17)

* Fix trait_documenter for properties (#1246)

* Add a failing tests

* And fix the failing test

(cherry picked from commit 464ec60)

* Update changelog

Co-authored-by: Kit Choi <kitchoi@users.noreply.github.com>
@mdickinson mdickinson added porting: backported to 6.1 This PR has been backported to the Traits 6.1.x release branch and removed porting: needs backport to 6.1 labels Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
porting: backported to 6.1 This PR has been backported to the Traits 6.1.x release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trait documenter fails on regular Python property s
2 participants