-
Notifications
You must be signed in to change notification settings - Fork 1
Datashader: Index page with reference to notebook #286
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
WalkthroughAdds a HoloViz link include; updates Dask docs to reframe “tutorial” as “usage”; introduces a new Datashader integration landing page; and rewrites the PyViz integration page to focus on hvPlot with adjusted links and layout. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/integrate/pyviz/index.md (1)
39-54: Consider reducing content duplication between PyViz and Datashader pages.Both
docs/integrate/pyviz/index.mdanddocs/integrate/datashader/index.mdinclude:
- The same Datashader example images (lines 39-44, ds1 and ds2)
- The same Learn section with
timeseries-datashader.mdcard include (lines 51-54)Since Datashader now has a dedicated page that's cross-referenced from this hvPlot section (Line 31), consider:
- Removing the duplicate images and Learn section from the PyViz page, or
- Having the PyViz page showcase hvPlot-specific examples instead
This would reduce maintenance overhead and provide clearer separation of concerns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/_include/links.md(1 hunks)docs/integrate/dask/index.md(2 hunks)docs/integrate/dask/usage.md(3 hunks)docs/integrate/datashader/index.md(1 hunks)docs/integrate/pyviz/index.md(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build docs
🔇 Additional comments (13)
docs/_include/links.md (1)
38-38: LGTM! HoloViz link addition is correctly formatted.The new link declaration follows the existing pattern and is alphabetically ordered between HoloViews and Indexing entries.
docs/integrate/datashader/index.md (3)
1-11: LGTM! Header and logo placement are well-structured.The page header follows standard documentation conventions with proper label definition, includes, and floating logo layout.
43-46: Learn section include verified:docs/_include/card/timeseries-datashader.mdexists and renders correctly.
31-36: Please run the following script to follow redirects and confirm the GitHub asset URLs resolve with HTTP 200:#!/bin/bash echo "Checking ds1 final status..." curl -I -L -s -o /dev/null -w "%{http_code}" "https://github.com/crate/crate-clients-tools/assets/453543/7f38dff6-04bc-429e-9d31-6beeb9289c4b" echo "" echo "Checking ds2 final status..." curl -I -L -s -o /dev/null -w "%{http_code}" "https://github.com/crate/crate-clients-tools/assets/453543/23561a87-fb4f-4154-9891-1b3068e40579"docs/integrate/pyviz/index.md (3)
8-8: LGTM! Logo sizing is appropriate.The PyViz logo height has been adjusted to 100px, providing better visual prominence on the page.
16-17: LGTM! Section rename aligns with documentation restructuring.Renaming from "hvPlot and Datashader" to just "hvPlot" makes sense given that Datashader now has its own dedicated page.
25-33: LGTM! Content refactoring improves documentation structure.The narrative now positions hvPlot as the primary focus while appropriately cross-referencing the dedicated Datashader page. The use of
{ref}datashaderensures proper linking.docs/integrate/dask/index.md (2)
29-29: LGTM! Cross-reference updated to match refactored label.The reference correctly points to the renamed
dask-usagelabel, maintaining consistency with the refactored documentation.
38-38: LGTM! Navigation label updated for clarity.Changing the toctree entry from "Tutorial" to "Usage" aligns with the refactored content terminology and provides clearer navigation.
docs/integrate/dask/usage.md (4)
1-1: LGTM! Label refactored for consistency.The label has been updated from
dask-tutorialtodask-usage, aligning with the terminology changes throughout the documentation.
5-6: LGTM! Introduction reframed as a usage guide.The wording has been updated from "tutorial" to "usage guide," which better describes the nature of the documentation content.
24-24: LGTM! Terminology consistency maintained.The reference to "tutorial" has been appropriately changed to "usage guide" in the Inserting data section.
194-194: LGTM! Conclusion updated with consistent terminology.The final reference to "tutorial" has been changed to "usage guide," completing the terminology refactoring throughout the document.
About
Continue adding integration guides from the community forum.
Preview
References