-
Notifications
You must be signed in to change notification settings - Fork 651
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
FEAT-#6983: Add Pluggable Documentation Module Support #6986
FEAT-#6983: Add Pluggable Documentation Module Support #6986
Conversation
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
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.
This PR doesn't change anything about the current behavior when it comes to docs, it only applies changes when the developer wants to modify the docs with this class.
We may want to use this in the future to have different docs for Ray, Dask, or other engines.
@@ -1093,6 +1093,18 @@ def isin(self, values): # noqa: PR01, RT01, D200 | |||
""" | |||
return super(DataFrame, self).isin(values) | |||
|
|||
def isna(self): |
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.
These had to be added to both Series
and DataFrame
because they share docs, but we may not want them to in the Documentation class.
In a separate PR, I think we should add the rest of these, but for now I have left it to these two as a proof of concept for what will be needed in the future. The tests will not pass if we do not have the distinction with Series
and DataFrame
.
Also, many jobs failed. Please take a look. |
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
Co-authored-by: Iaroslav Igoshev <Poolliver868@mail.ru>
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
Could you elaborate a bit on why we may want to have different docs Ray, Dask, or other engines? |
Some engines may have different expectations, particularly databases. If a database engine doesn't have the same guarantees as Ray or Dask, it should be reflected in the documentation. |
@sfc-gh-dpetersohn, got it, thanks for the clarification. CI is still failing, please take a look. For instance, lint (pydocstyle) failed because of the following error. D200: One-line docstring should fit on one line with quotes (found 3) |
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
Thanks for the ping @YarShev, I think I fixed everything (we will see if it passes 😄). It seems we are not super consistent with |
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
I see that we are really not consistent with |
@sfc-gh-dpetersohn, |
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
@YarShev finally figured out the issue |
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.
@sfc-gh-dpetersohn, LGTM, thanks!
What do these changes do?
Add support for custom documentation defined at runtime.
flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
git commit -s
docs/development/architecture.rst
is up-to-date