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.

Sign up for GitHub

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

Jump to bottom

Added docstrings to AIPlugIn #98

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added docstrings to AIPlugIn #98

wants to merge 1 commit into from

Conversation

asgibson
Copy link
Contributor

Here is an update for ai_plugin.py using Google style docstrings. Also using type annotations.

Is this the style we'd like to go forward with?
There are other style choices we could choose. i.e., NumPy, reST, or Epytext (others?)

Opinions?

@asgibson asgibson added documentation Improvements or additions to documentation enhancement New feature or request question Further information is requested convention Repository coding and documentation standards labels Dec 12, 2023
@asgibson asgibson self-assigned this Dec 12, 2023
@asgibson asgibson linked an issue Dec 12, 2023 that may be closed by this pull request
Add PEP compliant docstrings to ai_plugin #97
Open
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (822cd97) 100.00% compared to head (4430d0f) 100.00%.

Additional details and impacted files 
@@            Coverage Diff            @@
##              main       #98   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        25           
  Lines          938       937    -1     
  Branches       129       129           
=========================================
- Hits           938       937    -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@asgibson asgibson added the help wanted Extra attention is needed label Dec 12, 2023
the-other-james
the-other-james reviewed Dec 14, 2023
View reviewed changes
onair/src/ai_components/ai_plugin_abstract/ai_plugin.py
Superclass for data driven components: VAE, PPO, etc. Allows for easier modularity.
"""This serves as a base for all plugins.

This is the superclass for data driven components: VAE, PPO, etc. It is
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably update the examples to ones we have... which is none (the Kalman plugin doesn't count, right?)... so I don't know.

We could at least get rid of the acronyms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd be in favor of just removing that sentence for now.

@the-other-james
Copy link
Contributor

I don't have a strong preference for the comment style we use. If I had to pick, I would go with whatever the python libraries use.

@asgibson
Copy link
Contributor Author

We have decided to follow numpydoc style:
https://numpydoc.readthedocs.io/en/latest/format.html

@asgibson asgibson mentioned this pull request Apr 1, 2024
Merged
@dragonejt
Copy link
Contributor

Hello, I've been browsing the other PR's and issues in the repository. I just wanted to note that since you all have decided on a docstring style to move forward with, most IDEs now have support for automatically generating docstring formats/templates for you to fill in. Essentially, all you will have to do is type """ and press enter, and a docstring template will be generated based on your function parameters and return values. This will save you a lot of time vs typing out the docstrings in full by yourself.

In VSCode, you can use the autoDocstring extension, which has support for numpydoc.

PyCharm already has automatic docstring templates builtin, and already supports numpydoc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@the-other-james the-other-james the-other-james left review comments

@Evana13G Evana13G Awaiting requested review from Evana13G

@jlucas9 jlucas9 Awaiting requested review from jlucas9

@tjchase34 tjchase34 Awaiting requested review from tjchase34

@cfirth-nasa cfirth-nasa Awaiting requested review from cfirth-nasa

Assignees

@asgibson asgibson

Labels
convention Repository coding and documentation standards documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add PEP compliant docstrings to ai_plugin
4 participants
@asgibson @codecov-commenter @the-other-james @dragonejt