-
Notifications
You must be signed in to change notification settings - Fork 170
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] Add docs pointing to daft-launcher
#3369
base: main
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging #3369 will improve performances by 53.15%Comparing Summary
Benchmarks breakdown
|
Graphite Automations"Request reviewers once CI passes" took an action on this PR • (11/20/24)1 reviewer was added to this PR based on Andrew Gazelka's automation. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3369 +/- ##
==========================================
- Coverage 77.35% 76.73% -0.63%
==========================================
Files 685 685
Lines 83639 84764 +1125
==========================================
+ Hits 64703 65044 +341
- Misses 18936 19720 +784
|
For MVP it looks good to me, but I think there's a better way we can incorporate Daft Launcher in the Distributed Computing page because right now (besides looking at the left menu) users have to scroll all the way to the bottom to click to the section. Have you also thought about how/where to reference Daft Launcher on the Ray page? https://www.getdaft.io/projects/docs/en/stable/user_guide/integrations/ray.html |
I can add it there if needed. Just didn't know that that was something that we wanted. |
I'm just changing some docs! Argh! |
Hmm maybe just add something like "To run Daft on a distributed Ray cluster, see Daft Launcher [link to Daft Launcher docs on Daft docs]" wherever you see fit on the Ray page (maybe either right before "Usage" section or at the bottom of the page) |
f4f0ab8
to
c0b97c5
Compare
@@ -11,6 +11,7 @@ requirements.txt | |||
build | |||
.cython_build | |||
.hypothesis | |||
.ropeproject |
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.
What's this? We should avoid any autodoc thing that isn't standardized across the project I think, to avoid spurious refactors of the code when different developers work on the code
@@ -96,7 +96,7 @@ | |||
"learn/install": "../install.html", | |||
"learn/user_guides/dataframes": "intro-dataframes.html", | |||
"learn/user_guides/types_and_ops": "intro-dataframes.html", | |||
"learn/user_guides/remote_cluster_execution": "distributed-computing.html", | |||
"learn/user_guides/remote_cluster_execution": "user_guide/distributed-computing", |
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.
We should also add a redirect from the poweruser/distributed-computing
page to distributed-computing
I believe
|
||
.. toctree:: | ||
|
||
distributed-computing/daft-launcher |
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.
Let's avoid nesting, just make daft-launcher
a section here?
We can also verify your PR is doing what we want it to do here: https://deploy-preview-3369--daft-docs-preview.netlify.app/ |
Overview