Skip to content
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

Enable API reference docs to show accessor methods #44

Merged
merged 12 commits into from
Jun 21, 2024

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Jun 19, 2024

Previously, the API docs section of cupy-xarray has been disabled on because import cupy doesn't work on Readthedocs CI without CUDA. Luckily, there is now a cupy-core package on conda-forge (see conda-forge/cupy-feedstock#241) that makes CUDA optional, so we can use that to build the API docs!

Preview at https://cupy-xarray--44.org.readthedocs.build/api.html

cupy-xarray API Reference documentation page

(Yeah, not sure why some of those API methods are centre-aligned 😅)

I've also updated the 'Contributing documentation' section in the contributing guide which had some typos. Full steps to reproduce the docs build are:

conda env create --file ci/doc.yml
conda activate cupy-xarray-doc

cd docs
make html

References:

Fixes #5

weiji14 and others added 6 commits June 20, 2024 10:04
The cupy-core package from conda-forge does not bring in CUDA dependencies and thus can be installed on CPU-only CI services. Xref conda-forge/cupy-feedstock#229
Also fixed some typos around doc/docs.
See usage instructions in https://sphinx-autosummary-accessors.readthedocs.io/en/stable/usage.html. Also need to `import cupy_xarray` to fix `exception: no module named xarray.DataArray.cupy` error following xarray-contrib/sphinx-autosummary-accessors#107.
Fixes `AttributeError: type object 'CupyDatasetAccessor' has no attribute 'get'`
@weiji14 weiji14 mentioned this pull request Jun 19, 2024
@keewis
Copy link
Contributor

keewis commented Jun 19, 2024

(Yeah, not sure why some of those API methods are centre-aligned 😅)

I think that's because those don't have docstrings (DataArray.cupy.get() doesn't, either, but it shares the table with methods that do have one).

Edit: looks like all of them are centered, just that some of them have longer strings
Edit2: I'd also advise against using numpydoc and napoleon together, they don't always work well together
Edit3: to install the package, I'd follow the recommendation documented by RTD (add -e .. to the pip section in the environment file) – this is also what we're doing in xarray

@weiji14
Copy link
Member Author

weiji14 commented Jun 19, 2024

Edit2: I'd also advise against using numpydoc and napoleon together, they don't always work well together

Ok, removed numpydoc at commit 5087d89!

Edit3: to install the package, I'd follow the recommendation documented by RTD (add -e .. to the pip section in the environment file) – this is also what we're doing in xarray

Ah yes, I was trying with one dot ., but forgot that two dots .. are needed to move up a directory! Done at ccc1925 (and contributing instruction docs also updated to remove pip install --editable=. part).

Edit: looks like all of them are centered, just that some of them have longer strings

Hmm you're correct, they are all center-aligned. Let me try to play around with the docstrings and/or the sphinx template.

Adding some docstrings to the is_cupy, as_cupy and as_numpy  methods, so that the API docs page looks like center-aligned.
@weiji14
Copy link
Member Author

weiji14 commented Jun 19, 2024

Ok, couldn't figure out how to remove the center-alignment 😅, I had a look at other xarray-contrib repos using the Furo theme, and it seems like xwrf has the same issue, but cf-xarray doesn't.

So... I just added in the some docstrings to make it look less empty. Maybe @dcherian or someone else can look at cf-xarray's css file and figure out how to remove the centre-alignment issue.

@weiji14 weiji14 marked this pull request as ready for review June 19, 2024 23:51
@dcherian
Copy link
Contributor

Nice work @weiji14 I've added you as maintainer so you should be able to click merge here :)

re: center-aligning. I do not recall doing anything specific. Apparently flox has this issue too, i suspect it is a regression in furo

@weiji14
Copy link
Member Author

weiji14 commented Jun 20, 2024

re: center-aligning. I do not recall doing anything specific. Apparently flox has this issue too, i suspect it is a regression in furo

Ah ok, maybe best to report this upstream then at https://github.com/pradyunsg/furo? I'm not sure whether the API docs in flox or cf-xarray looked like this before, so can't tell if it's a regression bug. It looks as though Furo is centering tables by default, so maybe we need a directive to left align it, but I couldn't find out exactly how.

Nice work @weiji14 I've added you as maintainer so you should be able to click merge here :)

Thanks! You must have read my mind. Secretly, I've been hoping to get maintainer access after this PR because I have some free time next week, so maybe (just maybe) I'll put in some time to update #10 😈

Will leave this open a bit longer in case anyone has other suggestions, otherwise I'll go ahead and merge this sometime tomorrow.

@weiji14 weiji14 self-assigned this Jun 20, 2024
@weiji14 weiji14 added documentation Improvements or additions to documentation labels Jun 20, 2024
@weiji14 weiji14 merged commit c74ac17 into xarray-contrib:main Jun 21, 2024
4 checks passed
@weiji14 weiji14 deleted the docs/api branch June 21, 2024 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add API to docs build
3 participants