-
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
DOCS-#3904: Improving Modin README #3929
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3929 +/- ##
===========================================
- Coverage 85.59% 36.41% -49.18%
===========================================
Files 201 187 -14
Lines 16671 15893 -778
===========================================
- Hits 14269 5787 -8482
- Misses 2402 10106 +7704
Continue to review full report at Codecov.
|
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 tried to make suggestions where possible, but I think you'll have to fix the suggestions' formatting if you use them.
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.
Looks pretty good. I think @dorisjlee leaves a very good comment about demo-able code block straight away.
I'd probably mention explicitly somewhere that performance effect could better be seen on large datasets and on small data sets perf can be worse than pandas |
We also miss mentioning of current conda-forge version |
@naren-ponder : Once we finalize the README content (add GIF and TOC), can we also migrate the same content over to our home page for ReadTheDocs? |
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.
One change I would propose structurally is moving the "Choosing a Compute Engine + Which engine should I use?" sections right after "Installation" and before "Pandas API Coverage". I think this makes more logical sense since we discuss installing with different engines anyhow.
Additionally, for the sake of simplicity for the README I propose removing the "Advanced Usage" subsection concerning initializing Ray and Dask with custom resource constraints. Resource constrained installations are covered by https://github.com/modin-project/modin/pull/3965/files as part of using_modin_locally
in the documentation and I don't see a need for it in the README.
I also suggest retitling the "Full Documentation" section to "More about Modin". I also think we should retitle the "More information and Getting Involved" section at the end to "General Resources and Getting Involved", and adding links there to tutorials, blog posts, and other interesting links that would then be easily accessible directly from the README.
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.
A couple of comments, thanks @naren-ponder!
Signed-off-by: Naren Krishna <naren@ponder.io>
Signed-off-by: Naren Krishna <naren@ponder.io>
Co-authored-by: Mahesh Vashishtha <mvashishtha@users.noreply.github.com>
Signed-off-by: Naren Krishna <naren@ponder.io>
Signed-off-by: Naren Krishna <naren@ponder.io>
Signed-off-by: Naren Krishna <naren@ponder.io>
Signed-off-by: Naren Krishna <naren@ponder.io>
Co-authored-by: Doris Lee <dorisjunglinlee@gmail.com>
Signed-off-by: Naren Krishna <naren@ponder.io>
Co-authored-by: Doris Lee <dorisjunglinlee@gmail.com>
Signed-off-by: Naren Krishna <naren@ponder.io>
Signed-off-by: Naren Krishna <naren@ponder.io>
Signed-off-by: Naren Krishna <naren@ponder.io>
Signed-off-by: Naren Krishna <naren@ponder.io>
Signed-off-by: Naren Krishna <naren@ponder.io>
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.
An issue to be resolved should be linked in the PR description.
Signed-off-by: Naren Krishna <naren@ponder.io>
Signed-off-by: Naren Krishna <naren@ponder.io>
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.
@naren-ponder, LGTM, thanks! Do not forget to link an issue to be resolved by this PR in the PR description.
Signed-off-by: Naren Krishna <naren@ponder.io>
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.
LGTM!
Signed-off-by: Naren Krishna naren@ponder.io
What do these changes do?
Fix links in the Modin README and improve some of the wording in both the FAQ and README. Resolves #3904
flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
git commit -s
docs/developer/architecture.rst
is up-to-date