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

DOC: sphinx documentation #110

Merged
merged 14 commits into from
Jan 24, 2022
Merged

Conversation

martinfleis
Copy link
Member

Creating sphinx documentation available at readthedocs. At the moment contains autogenerated API documentation, bits and pieces from the ReadMe and the basic intro that is now in notebooks.

I've also slightly amended/added some docstrings.

The base for the docs (theme...) is copy&paste from GeoPandas.

@martinfleis
Copy link
Member Author

Now it works. See https://dask-geopandas--110.org.readthedocs.build/en/110/index.html

We're not executing the code at the moment (we do with geopandas) and I am not sure we want to. If we do, we need to ask for a higher memory quota, otherwise, we won't even build the env.

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! That looks good for an initial set up of the infrastructure.

One comment: I know this is somewhat copied from geopandas, but here I would not put part of the files in another "docs" subdir (so /docs/source/docs). As now the main page for the user guide is guide.html, but then the subpages are docs/guide/intro.html (which feels a bit strange, since guide.html is also part of the docs).
(in geopandas I think we did this because we also have more "non-docs" content on the website, so the docs was only one sub-part of the top-level navigation)

doc/source/_static/custom.css Outdated Show resolved Hide resolved
doc/source/docs/reference/geodataframe.rst Outdated Show resolved Hide resolved
@jorisvandenbossche
Copy link
Member

And I think it is fine to not execute the docs at the moment.
(although I am not sure if building the environment should be that much of a problem, it doesn't seem a more complex environment as for geopandas (eg we don't need cartopy here), and the package itself is only a small pure python package)

@martinfleis
Copy link
Member Author

but here I would not put part of the files in another "docs" subdir

That is fine for User Guide (I'll move that) but I need the same URL structure for API. Since we take docstrings from geopandas, I need the same depth from API pages to static files.

although I am not sure if building the environment should be that much of a problem, it doesn't seem a more complex environment as for geopandas

We have higher memory quota for GeoPandas from RTD.

@martinfleis
Copy link
Member Author

I have switched the theme to sphinx-book-theme and turned on the execution of notebooks. It seems to work fine now. The only issue is that we include a bit on performance in the introductory notebook that doesn't make sense now as we get probably 2? cores on rtd.

@martinfleis martinfleis added this to the 0.1 milestone Jan 17, 2022
@jorisvandenbossche
Copy link
Member

The only issue is that we include a bit on performance in the introductory notebook that doesn't make sense now as we get probably 2? cores on rtd.

Can we turn off execution for a specific notebook?
Or put that section in a formatted code block in a markdown cell with hardcoded results?

@jorisvandenbossche
Copy link
Member

For the rest looks good BTW! I think it's also fine to take this as a good start and get it merged.

@martinfleis
Copy link
Member Author

Can we turn off execution for a specific notebook?

We can rely on an automatic detection of notebook to execute, so if all cells will be executed, sphinx (mys-nb) will skip it. At the moment, I have set it to "force" so yes, we can turn it off. I'll push the change in a sec to make sure it was picked up by rtd correctly

@martinfleis
Copy link
Member Author

Looks okay now. I had to specifically list it as not to be executed as the auto option still executed it but it should be fine now.

@jorisvandenbossche jorisvandenbossche merged commit e953ff8 into geopandas:main Jan 24, 2022
@jorisvandenbossche
Copy link
Member

Thanks a lot!

@jorisvandenbossche
Copy link
Member

As a follow-up, we should probably update (and deduplicate) the README a bit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants