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

[TF-3443] Add pagination example #111

Merged
merged 3 commits into from
Jan 9, 2025
Merged

[TF-3443] Add pagination example #111

merged 3 commits into from
Jan 9, 2025

Conversation

jayfish0
Copy link
Contributor

@jayfish0 jayfish0 commented Jan 7, 2025

No description provided.

@jayfish0 jayfish0 changed the title add pagination example [TF-3443] add pagination example Jan 7, 2025
@jayfish0 jayfish0 changed the title [TF-3443] add pagination example Add pagination example Jan 7, 2025
Copy link
Contributor

@rachelnabors rachelnabors left a comment

Choose a reason for hiding this comment

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

Add a JS version

@jayfish0
Copy link
Contributor Author

jayfish0 commented Jan 8, 2025

Add a JS version

We currently do not have JS version of the pagination in SDK, shall we first land the pagination in python SDK along with its docs and then docs for JS after?

@jayfish0 jayfish0 requested a review from rachelnabors January 8, 2025 08:34
@jayfish0 jayfish0 changed the title Add pagination example [TF-3443] Add pagination example Jan 8, 2025
@rachelnabors
Copy link
Contributor

Hmm, IIRC it was agreed there would be parity between the two SDKs to keep them evolving alongside each other. cc @frankfeng98 @paveldudka is this feature launch ready before implementation in the JS SDK?

Copy link
Contributor

@frankfeng98 frankfeng98 left a comment

Choose a reason for hiding this comment

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

LGTM overall! If we could add more comments about pagination in the script. that will be great

from playwright.sync_api import sync_playwright

import agentql
from agentql.tools.sync_api import paginate
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a comment here about importing paginate module?

}
}
"""
paginated_data = paginate(page, QUERY, 3)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, can we add some comments to describe / introduce the method?

Copy link
Contributor

@frankfeng98 frankfeng98 left a comment

Choose a reason for hiding this comment

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

LGTM!

@jayfish0 jayfish0 dismissed rachelnabors’s stale review January 9, 2025 20:58

Will add JS later

@jayfish0 jayfish0 merged commit 7cfe24f into main Jan 9, 2025
1 check passed
@jayfish0 jayfish0 deleted the jason/paginate-data branch January 9, 2025 20:58
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