rapidsai / deployment

RAPIDS Deployment Documentation
https://docs.rapids.ai/deployment/stable/
9 stars 28 forks source link

point to new dask-cuda docs, ensure nightly deployment docs point to nightly RAPIDS API docs #403

Closed jameslamb closed 3 weeks ago

jameslamb commented 1 month ago

Contributes to #402.

dask-cuda docs were moved from readthedocs to docs.rapids.ai over a year ago (https://github.com/rapidsai/dask-cuda/pull/1211).

This removes the remaining links to that readthedocs site, in favor of directing to the new official home of dask-cuda docs.

While doing this, I also noticed that there are many places with hard-coded /stable or /nightly references to docs.rapids.ai/api. This proposes using templating to ensure that on the /nightly version of the deployment docs, those point to the /nightly version of RAPIDS docs.

cc @charlesbluca for awareness

review-notebook-app[bot] commented 1 month ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

jacobtomlinson commented 1 month ago

Unfortunately that doesn't look happy either

image
jameslamb commented 1 month ago

hmmm yeah, so I guess this:

https://github.com/rapidsai/deployment/blob/6eeb01ab8b451fb5d197917b997796281c718562/extensions/rapids_version_templating.py#L13

must run on the generated HTML, after it's already been rendered from markdown?

Because this is what I see in the page source for view-source:https://rapids-deployment--403.org.readthedocs.build/en/403/tools/kubernetes/dask-helm-chart/

[<code class="docutils literal notranslate"><span class="pre">dask_cuda_worker</span></code>](https://docs.rapids.ai/api/dask-cuda/nightly/index.html)

Would it be too weird to introduce some other, valid-in-a-URL, templating character for these cases? Like this?

https://docs.rapids.ai/api/dask-cuda/~~~rapids_api_docs_version~~~/index.html

And then modifying the replacement code to also replace that? I'm gonna try that.

jacobtomlinson commented 1 month ago

I like that idea! But maybe it looks like that implementation still needs the spaces.

jameslamb commented 1 month ago

Got pulled away from this near the end of the day, it is not ready to merge. I'll get the templating working tomorrow.

jacobtomlinson commented 1 month ago

No worries, I've moved it back to draft. Let me know when it's ready.

jameslamb commented 1 month ago

Ok @jacobtomlinson , I think I got this working!

As it turns out, none of these were correct:

These URLs weren't being templated because the docutils code in this project was not processing URLs at all!

The use of docutils.nodes.SparseNodeVisitor here...

https://github.com/rapidsai/deployment/blob/b6c222f8c29de49a14ebc7157c49b5d863a29983/extensions/rapids_version_templating.py#L6

... tells docutils "only visit document nodes for which the visitor has a corresponding visit_{node_class}() method defined". In our case here, that meant only Text nodes, because that was the only visit_{node_class}() method defined:

https://github.com/rapidsai/deployment/blob/b6c222f8c29de49a14ebc7157c49b5d863a29983/extensions/rapids_version_templating.py#L11

docutils stored links (like those that in HTML would be <a href="..."></a>) as a docutils.nodes.reference instance. I added a visit_reference() method on the class doing the templating, and implemented that method in a way that works with how docutils stores URLs.

Other changes I made, to hopefully make this easier for the next person:

I think we still want the use of a URL-valid templating string like `~~~, so that: