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

fix: Pager typing #1449

Merged
merged 3 commits into from
Sep 2, 2024
Merged

Conversation

jorwoods
Copy link
Contributor

Pager Protocols were missing the generic flags. Added those in so the Pager correctly passes through the typing information. Also removes the kwargs from the function signature of the Endpoint.get protocol to make the Workbook endpoint match.

Adding a return annotation of None to the tests is very important because it is what enables static type checkers, like mypy, to inspect those functions. With these annotations now in place, users of TSC should more transparently be able to carry through typing information when using the Pager.

Pager Protocols were missing the generic flags. Added those in
so the Pager correctly passes through the typing information.
Also removes the kwargs from the function signature of the Endpoint.get
protocol to make the Workbook endpoint match.

Adding a return annotation of `None` to the tests is very important
because it is what enables static type checkers, like mypy, to inspect
those functions. With these annotations now in place, users of TSC
should more transparently be able to carry through typing information
when using the Pager.
@jacalata
Copy link
Contributor

jacalata commented Sep 2, 2024

Not sure I understand your comment about adding the return type to the tests. How does that make it easier for users to get typing information in their code?

@jorwoods
Copy link
Contributor Author

jorwoods commented Sep 2, 2024

Making the protocols generic is what allows the pager to retrieve the typing information from the endpoint provided to Pager and surface that to users.

Adding a return type on tests is what gives mypy the ability to inspect the test functions. Since test functions somewhat mirrors how people would use the package, contributors to TSC should annotate test functions returning None to verify that typing information is available to the package consumers.

@jorwoods
Copy link
Contributor Author

jorwoods commented Sep 2, 2024

In this case, when I added typing to the Pager object in #1390, I missed annotating the functions as returning None. Since those test functions were not annotated, mypy falsely reported that there were no errors in typing.

@jacalata jacalata merged commit 4cf1916 into tableau:development Sep 2, 2024
22 checks passed
@jorwoods jorwoods deleted the jorwoods/fix_pager_typing branch September 2, 2024 19:30
jacalata added a commit that referenced this pull request Sep 17, 2024
v0.33

Features:
- add name, datasource-name to Job item
- enable bulk add and remove users
- Linked Tasks: get, get by ID, run Now
- implement Tags: create new, add/delete for workbooks, flows, datasources
- get page and chunk size from env vars
- add some repr implementations
- implement virtual connections

Bugfix:
- #1447
- #1449
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants