-
Notifications
You must be signed in to change notification settings - Fork 653
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
DOCS-#4401: Add supported APIs data for IO functions #4402
base: master
Are you sure you want to change the base?
DOCS-#4401: Add supported APIs data for IO functions #4402
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4402 +/- ##
==========================================
+ Coverage 86.36% 89.41% +3.05%
==========================================
Files 228 229 +1
Lines 18413 18689 +276
==========================================
+ Hits 15902 16711 +809
+ Misses 2511 1978 -533
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
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 we should add a description above the table - something like:
The following table highlights current support for parameters to this method. Parameters marked as Partial are partially supported, harmful are supported but may degrade performance ...
I agree, we should add some flags description, but if we add this description for each of the table, docstrings will be bloated, i think. My suggestion is to add flags description on the top of each file where tables with supported APIs data are used. What do you think about this approach? |
That makes sense to me! I think the description at the top should also include a blurb about how the table works, which will make it (a little) redundant across pages; but I think most users will only ever look at one page at a time, so a full description at the top will do more harm than good. If we do this, I think it would also be helpful to change the |
Ok, agree. Description was extended and docstring tag was changed to |
@RehanSD , Modin team, do you have any comments on this PR? |
e85b912
to
962413a
Compare
1bc68e1
to
2f94108
Compare
modin/pandas/base.py
Outdated
+-----------+-----------------------------------------------------------------------------------------------+ | ||
| Flag | Meaning | | ||
+===========+===============================================================================================+ | ||
| Harmful | Usage of this parameter can be harmful for performance of your application. Usually this | |
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.
Let's establish a name for this flag. Related discussion - link.
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.
cc @RehanSD, @devin-petersohn, take a look please
3c77430
to
a414fd7
Compare
e1df4c2
to
612526f
Compare
881b1e6
to
850009a
Compare
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Co-authored-by: Yaroslav Igoshev <[email protected]> Signed-off-by: alexander3774 <[email protected]>
Co-authored-by: Yaroslav Igoshev <[email protected]> Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
Co-authored-by: Yaroslav Igoshev <[email protected]> Signed-off-by: alexander3774 <[email protected]>
Co-authored-by: Rehan Sohail Durrani <[email protected]> Signed-off-by: alexander3774 <[email protected]>
Signed-off-by: alexander3774 <[email protected]>
850009a
to
4c74886
Compare
Signed-off-by: alexander3774 <[email protected]>
4c74886
to
e25302a
Compare
Signed-off-by: alexander3774 <[email protected]>
@RehanSD many thank for your very detailed review! P.S. Could you please help us to establish flag for perf degradation in the discussion? |
Hi @RehanSD! Please take a look at this PR. |
Hi Modin core team! Do you have any thoughts/comments on this PR? |
What do these changes do?
This PR adds supported APIs tables for IO functions that should be later used in #4286.
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