-
Notifications
You must be signed in to change notification settings - Fork 501
Add 5-min quick wins page to docs #3633
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
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Pull Request Overview
This PR enhances the documentation by updating an outdated link in the scheduled pipelines tutorial and adding a new "5-minute Quick Wins" page entry to the table of contents.
- Updated the URL in the scheduled pipelines tutorial to point to the correct scheduling reference.
- Added a new navigation link for a quick wins page in the table of contents.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
docs/book/user-guide/tutorial/managing-scheduled-pipelines.md | Updated the URL for schedule reference documentation. |
docs/book/user-guide/toc.md | Added the "5-minute Quick Wins" link under Best Practices. |
Comments suppressed due to low confidence (2)
docs/book/user-guide/tutorial/managing-scheduled-pipelines.md:26
- Please confirm that the updated URL reflects the current organization of the documentation and is fully aligned with the intended reference structure.
For our full reference documentation on schedules, see the [Schedule a Pipeline](https://docs.zenml.io/concepts/steps_and_pipelines/scheduling) page.
docs/book/user-guide/toc.md:62
- Ensure that the new '5-minute Quick Wins' page exists and that the link follows the established styling and hierarchy conventions in the table of contents.
* [5-minute Quick Wins](best-practices/quick-wins.md)
Documentation Link Check Results✅ Absolute links check passed |
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.
Great one! Sorry I didnt get to it in time.
I think I'd add the following:
- Add the Model for the Model Control Plane (seems like an obvious one most people don't use?
- Use tags as a way to organize artifacts and pipelines and run templates?
- Write simple smoke tests for remote pipelines on docker orchestrator
- Create a parent image to avoid image building headaches
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.
Pull Request Overview
This PR adds a quick wins documentation page to help users quickly grasp essential improvements.
- Updates the URL in the managing scheduled pipelines tutorial to the new docs location.
- Adds a new entry in the table of contents for the quick wins page.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
File | Description |
---|---|
docs/book/user-guide/tutorial/managing-scheduled-pipelines.md | Updated the reference URL for scheduling documentation. |
docs/book/user-guide/toc.md | Added a new bullet entry for the quick wins page. |
Comments suppressed due to low confidence (2)
docs/book/user-guide/toc.md:62
- [nitpick] Consider aligning the naming convention for the quick wins entry with the overall documentation style (e.g., '5-min' vs '5-minute') for consistency.
* [5-minute Quick Wins](best-practices/quick-wins.md)
docs/book/user-guide/tutorial/managing-scheduled-pipelines.md:26
- Verify that the updated URL is accurate according to the new documentation structure and that similar references are updated consistently.
For our full reference documentation on schedules, see the [Schedule a Pipeline](https://docs.zenml.io/concepts/steps_and_pipelines/scheduling) page.
@htahir1 addressed all your requests / comments in the latest version |
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.
Coming along - lets not rush the finer details :-) !
|
||
# Tag based on performance | ||
if accuracy > 0.9: | ||
add_tags(tags=["high-accuracy"], infer_artifact=True) |
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.
hmm what is infer_artifact?
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.
From the docstring: "Flag deciding whether the artifact version should be
inferred from the step context."
# Automatically attach to the current model | ||
log_metadata( | ||
{"accuracy": accuracy, "f1_score": 0.89}, | ||
infer_model=True # Automatically finds pipeline's model |
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.
am i the only one who hates these redundant ass flags?
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.
the infer_model
flag? it's useful if we only want to log metadata to the model and not the pipeline run... though it would be nice to be able to log to the model and pipeline run in one log_metadata
call.
RUN pip install --no-cache-dir \ | ||
zenml==0.54.0 \ | ||
tensorflow==2.12.0 \ | ||
torch==2.0.0 \ | ||
scikit-learn==1.2.2 \ | ||
pandas==2.0.0 \ | ||
numpy==1.24.3 \ | ||
matplotlib==3.7.1 | ||
|
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.
you also need to install stack requirements. here you can reuse some of the work you did in the tutorial you write for CI/CD
Images automagically compressed by Calibre's image-actions ✨ Compression reduced images by 36.4%, saving 117.15 KB.
342 images did not require optimisation. Update required: Update image-actions configuration to the latest version before 1/1/21. See README for instructions. |
…/zenml into doc/five-minute-quick-wins-page
Added instructions to install stack component requirements in the parent Docker image to ensure all required dependencies for the stack are available. This enhances the quick win by ensuring the parent image is fully prepared for all stack components. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
The absolute link checker failed while i was doing the apidocs thing . it should be fixed with redirects when DNS propogates BUT maybe its a good time to replace these links with sdkdocs anyway ?can you do this in this Pr please |
Yes already doing it. Pushing in a min and then will merge it in. |
* Fix link * Add quick wins guide + photo * Remove line breaks * Fix broken slack alerter link * Fixes based on PR feedback * Optimised images with calibre/image-actions * Addressing Marwan's comments * Handle installed docker * Add tag filtering image * Fix deleted text * Add stack requirements installation to parent Docker image guide Added instructions to install stack component requirements in the parent Docker image to ensure all required dependencies for the stack are available. This enhances the quick win by ensuring the parent image is fully prepared for all stack components. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * API & SDK docs fixes --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude <[email protected]> (cherry picked from commit 9285c4d)
* Add 5-min quick wins page to docs (#3633) * Fix link * Add quick wins guide + photo * Remove line breaks * Fix broken slack alerter link * Fixes based on PR feedback * Optimised images with calibre/image-actions * Addressing Marwan's comments * Handle installed docker * Add tag filtering image * Fix deleted text * Add stack requirements installation to parent Docker image guide Added instructions to install stack component requirements in the parent Docker image to ensure all required dependencies for the stack are available. This enhances the quick win by ensuring the parent image is fully prepared for all stack components. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * API & SDK docs fixes --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude <[email protected]> (cherry picked from commit 9285c4d) * Remove mention of "Other Services" connector type (#3646) (cherry picked from commit fc0ff9c) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude <[email protected]> Co-authored-by: Hamza Tahir <[email protected]>
Added a quick-wins page (as previously discussed) to our documentation.
Would welcome alternative / additional suggestions for things we could add here. Also potentially the sections could be a teeny bit shorter?
Feedback welcomed.