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 on spatial data #2750

Merged
merged 10 commits into from
Dec 24, 2022
Merged

documentation on spatial data #2750

merged 10 commits into from
Dec 24, 2022

Conversation

mattijn
Copy link
Contributor

@mattijn mattijn commented Dec 11, 2022

Fix #2740.

This PR introduces a different section how to specify data.

The following sections are defined (each on its own page):

  • DataFrame data
  • Dictionary data
  • URL data
  • Spatial data
  • Generator data

I only looked carefully to the page on spatial data, the others are merely infrastructure code that can be extended and improved by other PRs.

@mattijn mattijn requested a review from joelostblom December 11, 2022 22:34
Copy link
Contributor

@joelostblom joelostblom left a comment

Choose a reason for hiding this comment

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

Thanks for adding this section! I have some minor comments on wording and adding some links to other doc pages. I saw that you initially added the other data pages as well but then removed them, do you think we should add that (and remove the "specifying data" page) in a separate PR or add it to this one?

doc/user_guide/data/spatial.rst Outdated Show resolved Hide resolved
doc/user_guide/data/index.rst Outdated Show resolved Hide resolved
doc/user_guide/data/index.rst Outdated Show resolved Hide resolved
doc/user_guide/data/index.rst Outdated Show resolved Hide resolved
doc/user_guide/data/index.rst Outdated Show resolved Hide resolved
doc/user_guide/data/spatial.rst Outdated Show resolved Hide resolved
doc/user_guide/data/spatial.rst Outdated Show resolved Hide resolved
doc/user_guide/data/spatial.rst Outdated Show resolved Hide resolved
doc/user_guide/data/spatial.rst Outdated Show resolved Hide resolved
doc/user_guide/data/spatial.rst Outdated Show resolved Hide resolved
mattijn and others added 3 commits December 16, 2022 22:48
Love the review, thanks @joelostblom!

Co-authored-by: Joel Ostblom <joelostblom@users.noreply.github.com>
include mesh option for topojson
fix few typos
@mattijn
Copy link
Contributor Author

mattijn commented Dec 16, 2022

I saw that you initially added the other data pages as well but then removed them, do you think we should add that (and remove the "specifying data" page) in a separate PR or add it to this one?

I removed them again since I'd just copy+paste them into the different pages from the existing https://altair-viz.github.io/user_guide/data.html page. I did not yet remove this page, so I though it might probably be better that someone does this in a separate PR.

Probably good to check as well if these pages needs to be modified or extended.

@joelostblom joelostblom self-requested a review December 18, 2022 02:13
Copy link
Contributor

@joelostblom joelostblom left a comment

Choose a reason for hiding this comment

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

This looks great from what I can see! The Altair documentation has gotten SO much better in terms of geographic data with this and the mark_geoshape PR you made earlier. Thank you @mattijn ! Feel free to merge unless you were planning to add something else. I opened #2761 for the follow up data sections

@mattijn mattijn merged commit b246a5b into master Dec 24, 2022
@mattijn mattijn deleted the doc-spatial-data branch August 25, 2023 20:30
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.

Move the geospatial section in the "Specifying data" page to the geoshape mark page
2 participants