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

document Properties in class docstrings #2901

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

wakamex
Copy link
Contributor

@wakamex wakamex commented Feb 17, 2024

part one

@codecov-commenter
Copy link

codecov-commenter commented Feb 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.70%. Comparing base (af529ab) to head (3a9b575).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2901   +/-   ##
=======================================
  Coverage   96.70%   96.70%           
=======================================
  Files         147      147           
  Lines       14885    14885           
=======================================
  Hits        14395    14395           
  Misses        490      490           

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

@EnricoMi
Copy link
Collaborator

Is this auto-generated or hand-crafted?

@wakamex
Copy link
Contributor Author

wakamex commented Feb 22, 2024

good question. it does look very suspect.

a bit of both?

I started off by looking up the documentation for CommitStats because I wanted to know which stats are available. I was surprised that isn't listed in the documentation, so decided to add it. CommitStats is super vanilla, since it's just 3 items. Then I noticed none of the other classes documented their attributes in docstrings, so I started flipping through them.

To make that go faster I used GPT4 to generate descriptions from the _initAttributes, asking it to be concise, and reviewed them very briefly. I'm no expert on these objects, especially the more esoteric ones like CVE advisories, but they look right to me. I also focused on the most useful objects like PullRequest and Commit. I also re-ordered attributes in one case when they weren't in alphabetical order.

Despite the problems of genAI hallucination, in this case I'm fairly confident GPT4 has been trained on PyGithub (likely code and examples). To be 100% certain and avoid any risk of incorrect description hallucinated by these models, I can cut out all the docstrings except for CommitStats, which I'm confident in.

My thinking was some attribute documentation is better than none, especially when flipping through the Github object documentation here: https://pygithub.readthedocs.io/en/stable/github_objects.html.

@wakamex wakamex changed the title add docstrings document Properties in class docstrings Feb 22, 2024
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.

None yet

3 participants