vaexio / vaex

Out-of-Core hybrid Apache Arrow/NumPy DataFrame for Python, ML, visualization and exploration of big tabular data at a billion rows per second 🚀
https://vaex.io
MIT License
8.28k stars 590 forks source link

Clean up docs #2102

Closed NickCrews closed 2 years ago

NickCrews commented 2 years ago

Ideally we could treat doc warnings as errors so these things don't add up as they have been, but we're not there yet, warnings are still there.

NickCrews commented 2 years ago

Rebased on master, added in that fixup commit

JovanVeljanoski commented 2 years ago

Please ping us when you think this is ready for a review. You can also label it as draft until then. Good work tho!

NickCrews commented 2 years ago

@JovanVeljanoski Now that the tests pass this is ready for you to take a look at! Thanks.

NickCrews commented 2 years ago
  • When notebooks are changed, for various reasons a lot of metadata is also changed and thus committed.

Ugh, you're probably right, what a PITA. I use VSCode and so I didn't see the plaintext diff. Once we're happy with the actual content, I will do this. Don't want to do it too early though and then have to do it over and over again.

This would be a great thing for a tool that does this automatically, with a pre-commit hook and auto-fixer in CI...

@maartenbreddels is worried that this will lead to broken links

That's reasonable. Let me know. Possibly could use https://github.com/sphinx-contrib/redirects. But I would also say that the guides might evolve enough (eg swapping plot() to viz.heatmap()) that even if someone successfully can link to them from a 5 year old blog post, the contents of the guide are no longer relevant.

I responded to the individual review comments.

maartenbreddels commented 2 years ago

That's reasonable. Let me know. Possibly could use https://github.com/sphinx-contrib/redirects.

I'm not 100% sure how it works, but it shoulds like a good idea, better than broken external links.

NickCrews commented 2 years ago

@maartenbreddels so you want me to use sphinx-contrib/redirects?

Are there any other things at this point that you want me to change?

JovanVeljanoski commented 2 years ago

Yeah I think that is the idea to try out at least

maartenbreddels commented 2 years ago

Yes, that should be the only change needed for this PR.

NickCrews commented 2 years ago

I'm away from computer for another week, but I'll make that change when I'm back. Thanks for the clear actionable feedback!

On Wed, Jul 6, 2022, 2:50 AM Maarten Breddels @.***> wrote:

Yes, that should be the only change needed for this PR.

— Reply to this email directly, view it on GitHub https://github.com/vaexio/vaex/pull/2102#issuecomment-1176019954, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACSRYTRY3DUKE3YSRBX7MCDVSVJFNANCNFSM5ZYT3TBQ . You are receiving this because you authored the thread.Message ID: @.***>

NickCrews commented 2 years ago

OK @maartenbreddels @JovanVeljanoski I added a commit that does the redirects. It works, if you go to the RTD link from CI and try to go to /example_arrow.ipynb then it redirects to /guides/arrow.html. See if that is what you're looking for!

The one other point that I haven't heard @maartenbreddels explicitly say anything about are the deleted files:

docs/source/credits.rst docs/source/examples_notebooks.rst docs/source/gallery.rst docs/source/getting_data_in_vaex.rst docs/source/tipsandfaq.rst

See https://github.com/vaexio/vaex/pull/2102#discussion_r906866817 for my reasoning for deleting these. @maartenbreddels is this what you want? Want to merge this content into some other page or add it to a toctree?

maartenbreddels commented 2 years ago

The CI failure for 3.6 is unrelated.

maartenbreddels commented 2 years ago

The CI failure for 3.6 is unrelated.

If you feel like joining slack: https://join.slack.com/t/vaexio/shared_invite/zt-shhxzf5i-Cf5n2LtkoYgUjOjbB3bGQQ :)

NickCrews commented 2 years ago

Yay, thanks for the clear feedback and the support to get this merged quickly. It was a pleasant process to contribute :)

maartenbreddels commented 2 years ago

Good to hear, thanks!