-
Notifications
You must be signed in to change notification settings - Fork 10
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
docs: Added new getting started page #232
Conversation
…ge concepts and features pages
* master: chore: update dev deps (#227) chore: update dev deps and gh actions pipelines (#226) chore: Add Python Client examples [docs] (#191) Configure Renovate (#76) chore: bump version to 1.7.1 (#193) fix: aborting of last Actor/task run (#192) chore: Automatic docs theme update [skip ci] # Conflicts: # website/sidebars.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks!
I have a few code style suggestions to share.
On top of that, one more thing... It's not a requirement, more a personal preference. In many places I'd prefer not to chain so many calls into one command. Breaking it down to more separate commands can make it more understandable for users. Also when you want to transform such long-chained command into async command, you will end up with multiple awaits in one command. Which is not goood.
For example, instead of writing:
from apify_client import ApifyClient
apify_client = ApifyClient('MY-APIFY-TOKEN')
# Lists items from the Actor's dataset
dataset_items = apify_client.dataset('daset-id').list_items().items
I'd rather use something like:
from apify_client import ApifyClient
apify_client = ApifyClient('MY-APIFY-TOKEN')
# Get a dataset client
dataset_client = apify_client.dataset('daset-id')
# Lists items from the Actor's dataset
dataset_items = dataset_client.list_items().items
Co-authored-by: Vlada Dusek <v.dusek96@gmail.com>
@vdusek Thanks Vláďo for the suggested improvements! I added a couple of simplifications for the code examples (removed unnecessary chaining). Please let me know if it's ok now |
* master: chore: fix pytest deps (no module named pkg_resources) (#233)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just one nitpick, otherwise LGTM, thanks 🙂
Co-authored-by: Vlada Dusek <v.dusek96@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I tried to do the same thing as we did for the JS client here but for Python.
The goal is to improve onboarding (adaptation) for using our API (in Python in this case)