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 to be less fragile #1247

Merged
merged 10 commits into from
Jul 22, 2020

Conversation

mdickinson
Copy link
Member

@mdickinson mdickinson commented Jul 20, 2020

This PR modifies trait_documenter to be less fragile when a trait definition can't be found.

Previously, a failure to find a trait definition would raise StopIteration and cause the entire documentation build to be aborted. With the changes in this PR, that failure will instead issue a warning, and the documentation build will continue.

The PR also includes some refactoring to make it easier to add unit tests: in particular, the _get_trait_definition method, which depended on hard-to-set-up objects like the ClassLevelDocumenter, has had its logic moved into a standalone function trait_definition that doesn't need to know anything about Sphinx.

This PR fixes the aspect of #1238 that a failed definition retrieval breaks the doc build. It doesn't fix the underlying issue that caused the failed trait definition retrieval in the first place. That part is fixed by #1246. Note that the two PRs will almost certainly conflict, but resolving those conflicts shouldn't be particularly hard.

Also: #1246 should be backported to the release branch, while this PR is less bug-fixy, so should probably not be backported.

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 with nitpicky comments

traits/util/trait_documenter.py Show resolved Hide resolved
traits/util/trait_documenter.py Outdated Show resolved Hide resolved
traits/util/tests/test_trait_documenter.py Show resolved Hide resolved
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.

still LGTM

@mdickinson mdickinson merged commit 867433b into master Jul 22, 2020
@mdickinson mdickinson deleted the doc/refactor-trait-documenter branch July 22, 2020 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants