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

Reformat docstrings across Python client #219

Closed
wants to merge 3 commits into from

Conversation

austin-denoble
Copy link
Contributor

Problem

In a previous revision we added CI tooling for generating and deploying documentation for the Python client. #207

There was a lot of follow-up needed to clean up the generated documentation. A lot of our existing docstrings were also of disparate formatting types.

Solution

It was tricky aligning on a specific docstring format to go with. It seems like a lot of the auto-generated comments in /core/ are of either reST (reStructuredText, the sphinx default) or Google. A lot of the docstrings that were already stubbed out were of the Google style.

Since pdoc handles both types I went with Google as it felt a bit terser in terms of number of lines. Ultimately, I think most docs utilities offer plugins or options to handle either type.

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Non-code change (docs, etc)

Test Plan

Easiest way to validate the output would probably be to pull this branch down locally and run the pdoc utility:

$ poetry shell
$ poetry install

# same command we use in the `build-docs` action
$ poetry run pdoc pinecone/ '!pinecone.core' '!pinecone.info' --favicon ./favicon-32x32.png --docformat google -o ./docs

Once built, you can open the index.html file within the /docs/ folder.

@@ -0,0 +1,4 @@
# For testing purposes only
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to our TS client, I added in an example configuration file.

@@ -1,11 +1,13 @@
# pinecone-client
# pinecone-python-client
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the only thing I changed in here and it seems like there was some auto-formatting applied.

@@ -242,18 +243,71 @@ def init(
project_name: str = None,
log_level: str = None,
openapi_config: OpenApiConfiguration = None,
config: str = "~/.pinecone",
config: str = "./.pinecone",
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 don't think the previous default value worked for having the configuration file in the same location as the repo, but maybe the intent here was different? Tested and this works for me locally.

export PINECONE_CONTROLLER_HOST="your_controller_host"
```

**Using an INI configuration file**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Slipped a section in here for setting up an INI file. I don't think this was documented at all previously.

@jhamon jhamon deleted the adenoble/python-docstring-updates branch June 20, 2024 23:14
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.

1 participant