-
Notifications
You must be signed in to change notification settings - Fork 46
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
Classes Docstrings #5
Comments
Hey, I can fix this if you want. Will fix a PR for you. |
@frangiz - That'll be great. Thanks |
Issue #5: Added some docstrings for the new classes.
Hey there--is this still an issue that you'd like folks to help out with? I see that a pull request was merged about a year ago, but looking at the project today I still see a bunch of methods in the 'dyc' classes themselves that are missing docstrings and getting picked up by the tool:
If so, let me know and I can take a stab at adding docstrings for the above methods so |
Hey @demern - Please go ahead with it. I don't think anyone is looking at this currently. |
Good idea |
Hi @Zarad1993 - I've posted a PR for the new docstrings here: #61 I also filed a new issue for another bug I observed along the way: #62 |
Thanks @demern - I will try to move the PR forward today/tomorrow max. |
Hi @Zarad1993 , I'd like to contribute to this (for the classesBuilder() part). For this, what are you thinking for the format of a class docstring? Do we want something that has the parameters of the class itself (inheritance) or maybe the parameters of the init function? |
Yes, perhaps the parameters of the class inheritance would work perfectly.. We can iterate on it if you want too. |
Sounds good, we can start with just inheritance arguments in the docstring, and add to afterward. |
Hi @Zarad1993 , we sent a PR #68 . Can you review it when you get the chance? Thanks! |
@Zarad1993 This issue should be closed, right? |
Docstrings are missing in new clases and at the top of the file
The text was updated successfully, but these errors were encountered: