Skip to content

Conversation

jhamon
Copy link
Collaborator

@jhamon jhamon commented May 14, 2025

Problem

Python methods can be invoked with both positional and keyword arguments, however the keyword argument form has a number of benefits such as:

  • The keyword argument label acts as a minor form of documentation
  • Keyword arguments can be passed in any order, whereas positional arguments must be passed in a specific fixed order. This order flexibility works well when there are large numbers of optional parameters
  • If a function takes several parameters of the same type (e.g. str), it's very easy to accidentally pass them in the wrong order since you won't run into a type-related error letting you know you've done something wrong. Keyword args don't have this problem.
  • With positional arguments you must pass arguments with no default value before those that have a default value; this means you can't add a new default value without having to shuffle your argument order which creates a breaking change out of what should be a benign UX improvement. Keyword args do not have this limitation.

Solution

I recently implemented a decorator called @kwargs_required that will give an informative error if caller attempts to pass values as positional arguments.

This PR is a follow-up to apply that decorator to all methods that are newly added in the upcoming release. Adding this decorator would be a breaking change for existing methods, so for now I will hold off on doing that.

Type of Change

  • New feature (non-breaking change which adds functionality)

Testing

>>> pc.db.index.describe('foofoo')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/jhamon/workspace/pinecone-python-client/pinecone/utils/require_kwargs.py", line 10, in wrapper
    raise TypeError(
TypeError: describe() requires keyword arguments. Please use describe(name=value)
>>> pc.db.index.describe(name='foofoo')
{
    "name": "foofoo",
    "metric": "cosine",
    "host": "foofoo-dojoi3u.svc.aped-4627-b74a.pinecone.io",
    "spec": {
        "serverless": {
            "cloud": "aws",
            "region": "us-east-1"
        }
    },
    "status": {
        "ready": true,
        "state": "Ready"
    },
    "vector_type": "dense",
    "dimension": 2,
    "deletion_protection": "disabled",
    "tags": null
}

@jhamon jhamon changed the title Use @kwarg_required decorator on new methods Use @require_kwargs decorator on new methods May 14, 2025
@jhamon jhamon changed the title Use @require_kwargs decorator on new methods Use @require_kwargs decorator on new methods May 14, 2025
@jhamon jhamon force-pushed the jhamon/kwarg-required branch 2 times, most recently from 11f41fe to 154f95f Compare May 14, 2025 19:48
@jhamon jhamon force-pushed the jhamon/kwarg-required branch from 2f13526 to 5cb2197 Compare May 15, 2025 13:49
@jhamon jhamon marked this pull request as ready for review May 15, 2025 18:03
@jhamon jhamon merged commit 75d4bd3 into release-candidate/2025-04 May 15, 2025
70 checks passed
@jhamon jhamon deleted the jhamon/kwarg-required branch May 15, 2025 18:04
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