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

Documentation project #1700

Merged
merged 42 commits into from
Sep 5, 2023

Conversation

Conengmo
Copy link
Member

@Conengmo Conengmo commented Jan 11, 2023

See #1694.

Update: a first version can be seen https://python-visualization.github.io/folium/latest/index.html

In Github Pages let's do it as follows:

TODO:

  • fix broken links
  • publish the new docs
  • add version switcher
  • update Github Action to publish to dev and version folders (what this doesn't do is add the version switcher config)
  • take example data outside of Folium, store in separate repo
  • no longer do Selenium tests on example notebooks, only on new docs
  • push compiled docs to gh-pages 'latest' folder
  • redirect the main documentation index.html to the 'latest' folder
  • add redirects to gh-pages from old pages to new pages
  • merge this PR
  • update readme
  • have the example notebooks link to the new docs (new PR)

@Conengmo Conengmo marked this pull request as ready for review May 19, 2023 18:06
@Conengmo Conengmo changed the title Documentation project [WIP] Documentation project May 19, 2023
@martinfleis
Copy link
Collaborator

It is a super improvement over the existing version, thanks!

One question - do we mind that this breaks a lot of external links to existing docs? Since the structure is a bit different, the URLs that used to resolve will lead to 404. Sphinx has a nice tooling to avoid such breakage, here's how we did that in geopandas.

I might have a few follow-ups but better to merge this massive PR first :).

@Conengmo
Copy link
Member Author

Conengmo commented Jun 5, 2023

Thanks for your comment!

do we mind that this breaks a lot of external links to existing docs?

That's a great point, yes we do mind I think. Definitely a good idea to create a list of those moved pages. I'll include that in this PR. Then I'm interested to hear other follow-ups after this PR is out of the way, unless you have more comments that will affect the structure and urls.

@martinfleis
Copy link
Collaborator

No, nothing regarding structure. Just some sphinx plugins and similar things that are nice to have but not critical.

@ocefpaf
Copy link
Member

ocefpaf commented Jun 6, 2023

I didn't spend much time on writing additional new documentation yet. First get this migration out of the way.

Agreed. The changes here alone are complex enough!

Move necessary example data to the folium package: this way the data is included with the docs, there's no issue with paths and much less boilerplate to load them.

I have mixed feelings about this one. While I like the simplicity I'm afraid of the package size. Maybe, not in this PR, we can create an external data package or GH links and use them to build the docs. We can use pooch to manage the data.

If I get a go-ahead on this I'll let the main index.html point to the documentation in 'latest'.

👍 from me! Thanks for fixing years of my own laziness to improve the docs.

@martinfleis
Copy link
Collaborator

we can create an external data package

I am happy to include them in https://github.com/geopandas/geodatasets if we host them somewhere reliable (Github is fine), so we don't have to create another package for the same purpose.

@Conengmo
Copy link
Member Author

Conengmo commented Jun 21, 2023

These are good points. I guess it makes sense to not want to include the example data in the main repo.

What I'll do is create a repo in our org just with the selection of data files. That way we can load them by url, and they are not attached to our Folium releases.

I'm not convinced about Pooch: what I'd like is for users to be able to easily copy-paste the examples and run them, without having to install additional packages that are not a requirement for Folium. And I do think the current way of loading the data from a url is not that bad, Requests is already a dependency.

We could potentially include them in Geodatasets, though I don't think that will bring us much. Not all our data files are geojson or geometries even. Though I do think it's a cool library for our users to know about.

@Conengmo
Copy link
Member Author

Just realised I'll have to add those redirects to the root level manually after we've made the switch. Thanks for sharing that example Martin. I'll redirect these pages:

moved_pages = [
    ("installing.html", "latest/getting_started.html"),
    ("quickstart.html", "latest/getting_started.html"),
    ("flask.html", "latest/advanced_guide/flask.html"),
    ("modules.html", "latest/reference.html"),
    ("plugins.html", "latest/reference.html#module-folium.plugins"),
]

@Conengmo
Copy link
Member Author

@ocefpaf could you perhaps create a new empty example data repository in python-visualisation? I don't have sufficient permission for that. Or give me permission to do it. Thanks!

(I added this to an earlier comment but I'm afraid that doesn't notify you so I'm posting it separately)

@ocefpaf
Copy link
Member

ocefpaf commented Jun 21, 2023

@ocefpaf could you perhaps create a new empty example data repository in python-visualisation? I don't have sufficient permission for that. Or give me permission to do it. Thanks!

I also don't have permission for that. Only @wrobstory has.

@Conengmo
Copy link
Member Author

We then invoke the name of our creator, @wrobstory: rise and grant us permission to store our example data in a separate repository! 🪄 🌈

@Conengmo
Copy link
Member Author

I've sent him an email...

@wrobstory
Copy link
Contributor

hello

@ocefpaf and @Conengmo, you are both owners of the organization now, go forth and organize! 😂

Really amazing to see the continued work here, please let me know if there's anything else you need from my end

@Conengmo
Copy link
Member Author

Conengmo commented Aug 15, 2023

Thanks @wrobstory, really appreciate it!

@Conengmo
Copy link
Member Author

Conengmo commented Sep 5, 2023

I think we're ready to pull the trigger:

  • example data is now hosted in separate repo
  • Selenium tests run on the new documentation

Next is redirecting the main index.html to latest/index.html. And adding redirects the redirects listed here: #1700 (comment).

Latest docs are here: https://python-visualization.github.io/folium/latest

@ocefpaf @martinfleis let me know if you have additional comments before going ahead with this. If not I'll continue with the to do list in the PR description.

@martinfleis
Copy link
Collaborator

@Conengmo all good with me. Go ahead with the rest. Looking good!

@Conengmo
Copy link
Member Author

Conengmo commented Sep 5, 2023

Thanks Martin, appreciated. I'll go ahead then!

@ocefpaf
Copy link
Member

ocefpaf commented Sep 5, 2023

Same here. But I'm OK if you merge this one and keep working on a fresh PR to avoid confusion. This one already have 42 commits and touches 91 files!

@Conengmo
Copy link
Member Author

Conengmo commented Sep 5, 2023

But I'm OK if you merge this one and keep working on a fresh PR to avoid confusion

Yes, that's the plan. Hopefully the docs get updated automatically when merging to main 🤞

@Conengmo Conengmo merged commit 3cabb5a into python-visualization:main Sep 5, 2023
11 checks passed
@Conengmo
Copy link
Member Author

Conengmo commented Sep 5, 2023

Welp squashing that branch totally failed. Thanks Github. RIP our commit history

@Conengmo
Copy link
Member Author

Conengmo commented Sep 5, 2023

Hopefully the docs get updated automatically when merging to main

It actually did work 🎉

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.

4 participants