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

map.rst doumentation added #554

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

map.rst doumentation added #554

wants to merge 2 commits into from

Conversation

gp0121
Copy link

@gp0121 gp0121 commented Sep 11, 2022

Formatted map.rst to provide documentation to the maps.py methods similar to the tables.rst. Summarizes the purpose of each function in the classes in maps.py to separates it into creation, mutation, accessing, and transformation.

.. autosummary::
:toctree: _autosummary

_FoliumWrapper.draw
Copy link
Member

Choose a reason for hiding this comment

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

What is the rationale for including docs on _FoliumWrapper? Normally classes that start with an underscore are not intended to be a primary part of the visible API?

.. autosummary::
:toctree: _autosummary

Map.__getItem__
Copy link
Member

Choose a reason for hiding this comment

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

Should methods starting with an _ be surfaced here?

.. autosummary::
:toctree: _autosummary

Map._set_folium_wrap
Copy link
Member

Choose a reason for hiding this comment

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

Should this method be made visible in this documentation?


**Summary of Marker Class**

The Marker class instantiates a marker that is displayed with _FoliumWrapper's simple_marker method. The color of the marker can either be chosen from [‘red’, ‘blue’, ‘green’, ‘purple’, ‘orange’, ‘darkred’,'lightred’, ‘beige’, ‘darkblue’, ‘darkgreen’, ‘cadetblue’, ‘darkpurple’,
Copy link
Member

Choose a reason for hiding this comment

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

The fact that it uses _FoliumWrapper doesn't seem like a critical thing to document, that users need to understand.

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