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

MAINT: Get docs to build again #384

Merged
merged 4 commits into from
Nov 4, 2017
Merged

MAINT: Get docs to build again #384

merged 4 commits into from
Nov 4, 2017

Conversation

jvkersch
Copy link
Contributor

To get the docs build back on the rails (#380), I reverted the Python 2/3 compatibility layer from #369 for the Sphinx extensions (the rest of the compatibility layer is still in place). I began by updating the import statements to match what Sphinx expects, but even after doing this I found that some of the six constructs didn't play well with the rest of the code. As this code is old and will not work on Python 3 without some serious maintenance (#324), I decided to just roll back that part of the code.

closes #380

Copy link
Contributor

@corranwebster corranwebster left a comment

Choose a reason for hiding this comment

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

Some changes which really shouldn't have been reverted from the point of view of Python 2.7. Fixing is optional.

What would the work be to eg. get rid of Numpydoc in favour of Napoleon?

Similarly what would the work level be to get rid of the Traits documenter in favour of traits.util.trait_documenter or https://github.com/enthought/trait-documenter (not sure which is currently preferred).

@@ -441,8 +437,8 @@ def __str__(self):
'meth': 'method'}

if self._role:
if self._role not in roles:
print("Warning: invalid role %s" % self._role)
if not roles.has_key(self._role):
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems an unnecessary change.

@@ -419,7 +415,7 @@ def __init__(self, func, role='func'):
argspec = inspect.formatargspec(*argspec)
argspec = argspec.replace('*','\*')
signature = '%s%s' % (func_name, argspec)
except TypeError as e:
except TypeError, e:
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't revert this line!

#print("Docstring follows:")
#print(doclines)
#print('='*78)
except ValueError, e:
Copy link
Contributor

Choose a reason for hiding this comment

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

Or this line!

@@ -282,7 +275,7 @@ def _import_by_name(name):
return obj
else:
return sys.modules[modname]
except (ValueError, ImportError, AttributeError, KeyError) as e:
except (ValueError, ImportError, AttributeError, KeyError), e:
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, keep the 'as'

@jvkersch
Copy link
Contributor Author

jvkersch commented Nov 3, 2017

Thanks Corran. Just for the record, the reverts were done completely mechanistically. I'd rather not touch this part of the codebase for the time being; I agree that using standard Sphinx plugins like Napoleon and the Traits documenter (which were probably not around when this code was originally written) is the way forward.

@jvkersch jvkersch merged commit 6088e4a into master Nov 4, 2017
@jvkersch jvkersch deleted the maint/docs-python-27 branch November 4, 2017 15:38
jvkersch added a commit that referenced this pull request Nov 14, 2017
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.

Docs don't build
2 participants