-
Notifications
You must be signed in to change notification settings - Fork 119
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
SNOW-1320543 Merge back Modin documentation #1427
Conversation
session | ||
io | ||
general_functions | ||
modin.pandas.Series <series> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plan is to try to have all of the modin documentation displayed as modin.pandas.<class>
instead of snowflake.snowpark.modin.pandas.<class>
- this is a WIP and I'll introduce that in a new PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add those files' ownership?
@@ -59,7 +57,6 @@ | |||
# This pattern also affects html_static_path and html_extra_path. | |||
exclude_patterns = [] | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the snowflake_rtd_theme
part of conf.py
from the Modin version of the docs, removed the entire snowflake_rtd_theme
since it is no longer used.
return ( | ||
f"https://github.com/snowflakedb/snowpark-python/blob/" | ||
f"v{release}/{os.path.relpath(fn, start=os.pardir)}{linespec}" | ||
f"release-v{release}/{os.path.relpath(fn, start=os.pardir)}{linespec}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we add the release
prefix here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think snowpandas was working with an older version of this file - looks like the "release" part was removed a month ago, I can get rid of it to keep up with the new change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should stick with the Snowpark Python version, since I don't think this link ever was used by Snowpark pandas in the first place.
converting to draft PR - will split this PR into two. A different PR will first move the snowpark docs into its own folder. Then this PR will be used to merge in the modin changes. |
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes #SNOW-1320543
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
Adding documentation for all modin code. Moved Snowpark documentation into a
snowpark/
directory for better organization. Copiedmake view
command from Snowpark pandas fork.