pola-rs / r-polars

Bring polars to R
https://pola-rs.github.io/r-polars/
Other
405 stars 35 forks source link

Prepare docs for `altdoc` 0.3.0 #540

Closed etiennebacher closed 5 months ago

etiennebacher commented 6 months ago

altdoc 0.3.0 is going to be a massive change, most of existing things will have to updated. Here's a list of the most important changes in the repo, but there shouldn't be any visual changes on the website:

We're still working on altdoc so this PR is only to collect bugs to fix in altdoc (this is also why I deactivated the other CI for now)

etiennebacher commented 6 months ago

Still need to fix the r-universe badge -> upstream issue in altdoc

etiennebacher commented 5 months ago

@eitsupi I have some trouble with detecting the right python installation in the workflow.

At the end of altdoc::render_docs(), we call mkdocs build to finalize the documentation. The problem is that mkdocs is not found here. It looks like it is installed in the .venv folder (as expected) but the package is not found by altdoc because it doesn't use the good python installation:

image

image

In altdoc, we try to automatically find the right python installation with which python. This works for Windows (it correctly shows the .venv folder) but not for Unix because which python returns a path starting with /opt/:

image

(in the image above, .python_installation() is the altdoc function that runs which python)

Do you know a way in Unix to automatically list the .venv folder in the output of which python (as is the case in Windows)? Let me know if this is unclear

eitsupi commented 5 months ago

See https://docs.python.org/3/library/venv.html#how-venvs-work

$ source .venv/bin/activate
$ which python
/workspaces/r-polars/.venv/bin/python
etiennebacher commented 5 months ago

Thanks @eitsupi but it doesn't work. For some reason I get a sh: 1: source: not found. I tried to add shell: bash in the CI file and to run sudo -s (recommended here) but no effect

eitsupi commented 5 months ago

Where do you define that command? Use Bash instead of sh, or use . instead of source.

etiennebacher commented 5 months ago

I run system2("source", ".venv/bin/activate") in altdoc::render_docs(). It runs from the root of the package but I also tried with the absolute path to activate and neither works.

This is in a special branch "debugging" in altdoc: https://github.com/etiennebacher/altdoc/blob/9c7eb11eea6dec8622a1057a024a3410f06795c6/R/settings_mkdocs.R#L74

eitsupi commented 5 months ago

@etiennebacher I think you should use bash. https://stackoverflow.com/questions/40845452/specify-which-shell-to-use-in-r

eitsupi commented 5 months ago

I don't know why you're trying to do it from within R, but note that you have to use python commands within the session you've activated.

etiennebacher commented 5 months ago

Thanks I'll try with that

etiennebacher commented 5 months ago

I don't know why you're trying to do it from within R, but note that you have to use python commands within the session you've activated.

The objective is that the user just has to run altdoc::render_docs() to create the docs folder, ready to be pushed / previewed. So we need to call mkdocs build at the end of the function

eitsupi commented 5 months ago

Common sense tells me that activating Python from altdoc's function is pretty weird. I have to sleep and I'm on a business trip for the next week, so I don't have time to look at this, but I'm not sure if I'm going in the right direction. Shouldn't it be the user who enables venv because we don't know where the user's python is. User may use mamba or pixi or rye or...

eitsupi commented 5 months ago

And, running the sudo command by default is a really bad idea. It almost always should fail

etiennebacher commented 5 months ago

And, running the sudo command by default is a really bad idea.

Yeah I thought so too but I was a bit desperate about how to fix this. I need to think more about the python detection when we use mkdocs because finding the best installation to use is indeed challenging

eitsupi commented 5 months ago

I recommend that you do not try to handle everything with R functions, but rather to call some command from within a shell activated with venv, etc.

eitsupi commented 5 months ago

@etiennebacher Perhaps you are mistaken about venv activation and shell sessions.

Compare the following two commands:

$ bash -c 'source .venv/bin/activate' && bash -c 'which python3'
/usr/bin/python3
$ bash -c 'source .venv/bin/activate && which python3'
/workspaces/r-polars/.venv/bin/python3

You cannot call python in venv unless you are in a session that has activated venv.

etiennebacher commented 5 months ago

@eitsupi I think this is ready. I pinned the altdoc version because we're still working on it and I want to avoid accidental failures here. I checked locally and the website looks the same as the current version. When this is merged, I can start addressing all the issues on documentation

etiennebacher commented 5 months ago

Alright I'm merging now. Many thanks for your help with python / venv, I think the altdoc workflow with mkdocs is now more robust