Skip to content

Refactor function signatures to use kwargs #1141

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

Open
asonnenschein opened this issue May 20, 2025 · 1 comment
Open

Refactor function signatures to use kwargs #1141

asonnenschein opened this issue May 20, 2025 · 1 comment
Assignees
Labels
Milestone

Comments

@asonnenschein
Copy link
Contributor

Problem
A lot (most?) of the methods in the Python SDK use positional arguments in their signatures, which has proven to be prohibitive to maintaining feature parity between the SDK and Planet APIs when we need to add, remove, and/or change an argument from required to optional (or vice versa). For example, Subscriptions API and Orders API have a source type parameter that used to be a required parameter, but is now optional. Some SDK methods expect source type as a positional argument, sometimes in the first position. Making the source type argument in the SDK then becomes a breaking change, so we have to keep it and make it nullable which is an odd pattern. See #1107 for a real example of this issue.

Solution
Explore refactoring positional arguments in function signatures to keyword arguments, aka 'kwargs'. Python kwargs don't need to maintain order because their descriptor is the keyword. This means that moving forward we would be able to add, remove, change required parameters in the SDK to maintain feature parity with Planet APIs without introducing breaking changes to users.

A/C

  • Refactor all Planet API specific positional arguments in SDK methods to keyword arguments (e.g. source_type in Subscriptions API, feature_ref in Features API, etc).
  • Positional arguments that should never change (e.g. an HTTP request positional argument to a method that wraps an HTTP request) do not need to be converted to kwargs.
  • Update all tests that will be effected by this change.
  • Update docs.
@asonnenschein asonnenschein added this to the 3.0.0 Release milestone May 20, 2025
@asonnenschein asonnenschein self-assigned this May 20, 2025
@ischneider
Copy link
Member

I took a stab at adopting https://peps.python.org/pep-0570/ in the mosaics API PR - for example: https://github.com/planetlabs/planet-client-python/pull/1131/files#diff-8970078d5199e6cc25467d16698e6ef3f0ac91b131c0f511d46eaa0f3140638dR162

I agree with refactoring as much as possible but find positional parameters acceptable, even desirable, when it's obvious one or two parameters are required for a function, e.g. do not make be call len(obj=text) - similarly, listing features from a collection will always required the collection identifier but making optional filter arguments positional would be a bad design choice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants