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

Add some more docstrings. #536

Merged
merged 10 commits into from
Jul 18, 2014
Merged

Add some more docstrings. #536

merged 10 commits into from
Jul 18, 2014

Conversation

markkness
Copy link
Contributor

This PR adds some more docstrings to distarray, to get better coverage as per issue #525.

It also makes some pep8 adjustments, where I felt like following its rules. A lot of the added docstrings are for exception classes, and as I was investigating these, several seemed to be unused. In those cases, instead of adding a docstring (since there was no usage to suggest the correct text), I added a comment that the exception class seemed to be unused. If they really are unused, then perhaps they should just be removed from the code base. But I did not do that here so far.

@@ -25,10 +25,12 @@


class InvalidGridShapeError(Exception):
Copy link
Contributor

Choose a reason for hiding this comment

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

Side question: should these Error classes should be in error.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would make sense to me. I didn't try and move them in this PR though.

@bgrant
Copy link
Contributor

bgrant commented Jul 18, 2014

I'm fine with removing the unused Error classes... I seem to remember that Kurt was in favor of this too. @kwmsmith ?

@bgrant bgrant added this to the 0.5 milestone Jul 18, 2014
@markkness
Copy link
Contributor Author

I have removed the unused exception classes. The tests still pass locally.

@bgrant
Copy link
Contributor

bgrant commented Jul 18, 2014

Looks good to me, and builds without errors.

bgrant added a commit that referenced this pull request Jul 18, 2014
@bgrant bgrant merged commit 8801ca0 into master Jul 18, 2014
@bgrant bgrant deleted the doc-sprint-mkness branch July 18, 2014 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants