Skip to content

Conversation

@cjumel
Copy link
Contributor

@cjumel cjumel commented Sep 17, 2025

Description

This PR brings two quite important changes to the SDK, first around parameter management:

  • use the same order as in the official documentation for the entrypoint parameters (this can break a workflow using the very last parameters of the entrypoints as positional arguments, but this is not very likely)
  • defer the default parameter choice to the API (this won't change anything as of today, in the future, if an API default parameter change, then the new versions of the SDK will use that, vs the old one will stick to their default)

and around dependency management:

  • specify minimal version of the dependencies (they have been chose quite low so they shouldn't be too restrictive)

In the meantime, I updated the various docstrings to be more accurate and follow more systematically the google docstring convention.

Checklist

  • I have installed pre-commit on this project (for instance with the make install-dev command)
    before creating any commit, or I have run successfully the make lint command on my changes
  • I have run successfully the make test command on my changes
  • I have updated the README.md if my changes affected it

This commit performs a quite significant update of the SDK:
- make sure the LinkupClient has the same parameter ordering as in the
API documentation (might be breaking for users using only positional
arguments)
- delegate the choice of the default parameters to the API instead of
re-implementating them (should not be breaking anything now, might be
breaking in the future if the API defaults change themselves)
- update the documentation
This minimal versions should not be very constraining, since they are
quite old, but in theory this is still a breaking change.
@cjumel cjumel requested a review from vlapparov September 17, 2025 15:32
Copy link

@vlapparov vlapparov left a comment

Choose a reason for hiding this comment

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

LGTM. Not sure about the test case- need some clarification

@cjumel cjumel merged commit 9aaf6ae into main Sep 18, 2025
2 checks passed
@cjumel cjumel deleted the cj/defaults branch September 18, 2025 08:30
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.

3 participants