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.

Sign up for GitHub

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

Jump to bottom

Update Documentation with Pydata Sphinx Theme, and more #523

Merged
merged 9 commits into from
May 30, 2022
Merged

Update Documentation with Pydata Sphinx Theme, and more #523

merged 9 commits into from
May 30, 2022

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented May 11, 2022

This PR closes #459 . It updates the sphinx theme with Pydata-sphinx-theme.

Besides this PR also:

  • Fixed all existing warnings emitted during rendering the doc
  • Added a new section: internals.rst to focus on discussing GeoArrow format. Shuffled around the docstrings for GeoArrowBuffer and make background description and API usage relatively independent, but all presented in the same page for shared context.
  • Minor improvement to from_geopandas docstring.

@isVoid isVoid requested review from a team as code owners May 11, 2022 00:28
@github-actions github-actions bot added conda Related to conda and conda configuration Python Related to Python code labels May 11, 2022
@isVoid isVoid added documentation doc Documentation 3 - Ready for Review Ready for review by team non-breaking Non-breaking change improvement Improvement / enhancement to an existing function and removed conda Related to conda and conda configuration Python Related to Python code doc Documentation labels May 11, 2022
@isVoid
Copy link
Contributor Author

isVoid commented May 11, 2022

Front page:
image

harrism
harrism requested changes May 11, 2022
View reviewed changes
Copy link
Member

@harrism harrism 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 cleaning up the docs and updating to the new format @isVoid ! I have some edit suggestions for the docs while you are at it.

docs/source/api_docs/geopandas_compatibility.rst Outdated Show resolved Hide resolved
docs/source/api_docs/geopandas_compatibility.rst Show resolved Hide resolved
docs/source/api_docs/gis.rst Outdated Show resolved Hide resolved
docs/source/api_docs/internals.rst Outdated Show resolved Hide resolved
docs/source/api_docs/internals.rst Outdated Show resolved Hide resolved
docs/source/api_docs/spatial_indexing.rst Outdated Show resolved Hide resolved
docs/source/api_docs/trajectory.rst Outdated Show resolved Hide resolved
docs/source/index.rst
Comment on lines +18 to +19
cuSpatial proposes a new GeoArrow format from the fruit of discussions
with the GeoPandas team. GeoArrow is a packed columnar data format
Copy link
Member

Choose a reason for hiding this comment

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

GeoArrow exists independently of cuSpatial. This makes it sound like we invented GeoArrow. I don't think this sentence belongs in our documentation. (CC @thomcom )

Suggested change
cuSpatial proposes a new GeoArrow format from the fruit of discussions
with the GeoPandas team. GeoArrow is a packed columnar data format
GeoArrow is a packed columnar data format

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fine. I had the first implementation, though GeoArrow diverged from my implementation a little as per the previous discussions. :D :D

docs/source/index.rst Outdated Show resolved Hide resolved
python/cuspatial/cuspatial/core/interpolate.py Outdated Show resolved Hide resolved
ajschmidt8
ajschmidt8 approved these changes May 11, 2022
View reviewed changes
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

Approving ops-codeowner file changes

@isVoid @harrism
Various updates to docs wordings.
2920ce0 
Co-authored-by: Mark Harris <mharris@nvidia.com>
@github-actions github-actions bot added conda Related to conda and conda configuration Python Related to Python code labels May 11, 2022
@isVoid
Copy link
Contributor Author

isVoid commented May 12, 2022

Thanks to @ajschmidt8 , the documentation is now live for review at https://627d5be4c229560ba620c84d--docs-rapids-ai.netlify.app/api/cuspatial/nightly/ !

This was referenced May 12, 2022
Closed
Merged
@harrism harrism requested a review from thomcom May 20, 2022 01:00
@harrism
Copy link
Member

harrism commented May 20, 2022

@thomcom could you please review this PR?

@caryr35 caryr35 added this to PR-WIP in cuSpatial v22.06 Release via automation May 25, 2022
@caryr35 caryr35 moved this from PR-WIP to PR-Needs review in cuSpatial v22.06 Release May 25, 2022
thomcom
thomcom approved these changes May 27, 2022
View reviewed changes
docs/source/api_docs/internals.rst
Comment on lines +74 to +87
"mlines":
[1, 3]
})

Which encodes a GeoPandas Series::

series = geopandas.Series([
LineString((0, 0), (0, 1), (0, 2)),
MultiLineString([(1, 0), (1, 1), (1, 2)],
[(2, 0), (2, 1), (2, 2)],
)
LineString((3, 0), (3, 1), (3, 2)),
LineString((4, 0), (4, 1), (4, 2)),
])
Copy link
Contributor

Choose a reason for hiding this comment

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

Heya, sorry for not being in this convo earlier. I'll be looking into dropping the 'bounding regions' in the mlines object to fit GeoArrow format correctly.

cuSpatial v22.06 Release automation moved this from PR-Needs review to PR-Reviewer approved May 27, 2022
@harrism
Copy link
Member

harrism commented May 30, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 38d9e50 into rapidsai:branch-22.06 May 30, 2022
cuSpatial v22.06 Release automation moved this from PR-Reviewer approved to Done May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomcom thomcom thomcom approved these changes

@harrism harrism harrism approved these changes

@ajschmidt8 ajschmidt8 ajschmidt8 approved these changes

Assignees
No one assigned
Labels
3 - Ready for Review Ready for review by team conda Related to conda and conda configuration improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Related to Python code
Projects
No open projects
cuSpatial v22.06 Release
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[DOC] Update cuSpatial documentation to use PyData Sphinx theme
4 participants
@isVoid @harrism @thomcom @ajschmidt8