FEAT-#3044: Create Extentions Module in Modin #6961
Conversation
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
devin-petersohn
requested review from
mvashishtha,
RehanSD,
YarShev,
vnlitvinov,
anmyachev,
dchigarev and
a team
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
devin-petersohn
changed the title
YarShev
changed the title
modin/extension/__init__.py
Outdated
| @@ -0,0 +1,11 @@ | |||
| from .extensions import ( | |||
There was a problem hiding this comment.
Should we move modin/extension to modin/pandas/api/extensions?
There was a problem hiding this comment.
I am imagining eventually adding API extensions for numpy and even others.
There was a problem hiding this comment.
@sfc-gh-dpetersohn Users will most likely look for these functions in the same import path as in pandas. I think this is a good enough reason to make from modin.pandas.api.extensions import register_* work.
There was a problem hiding this comment.
If we wanted to add other API extentions, say for numpy, we would put them in modin.numpy.api.extensions. @sfc-gh-dpetersohn, what do you think?
There was a problem hiding this comment.
Sure, we can move it.
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>
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>
|
Just in case, CI is failing for now and we are working on that in #6969. |
There was a problem hiding this comment.
@devin-petersohn, LGTM, but let's wait for #6969 to get in first. Do you want to add a doc page about the extentions in a separate PR?
|
Sounds great @YarShev. I think we can leave this undocumented for the moment since I put some extensive docstrings in the PR. Eventually those should be the docs, but we can enable it in a separate PR. |
|
@devin-petersohn, could you rebase on current master to rerun CI? It should pass after that. |
.github/workflows/ci.yml
Outdated
| @@ -650,6 +650,7 @@ jobs: | |||
| mkdir -p "${CONDA_PKGS_DIR}_do_not_cache" && \ | |||
| find "${CONDA_PKGS_DIR}" -mindepth 1 -maxdepth 1 -type d -exec mv {} "${CONDA_PKGS_DIR}_do_not_cache" \; | |||
| if: matrix.os == 'windows' | |||
| - run: ${{ matrix.execution.shell-ex }} modin/extensions/test | |||
There was a problem hiding this comment.
@devin-petersohn you added a test to the place where ray was already stopped. You need to add running tests a couple of lines higher.
There was a problem hiding this comment.
Fixed, thank you!
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
|
This is finally passing and ready for final review! |
Signed-off-by: Devin Petersohn <devin.petersohn@snowflake.com>
Signed-off-by: Igoshev, Iaroslav <iaroslav.igoshev@intel.com>
|
@sfc-gh-dpetersohn, I had some more questions on the changes but not to waste time I went ahead and pushed some changes. Hope, it is fine to you. |
|
Yes, of course this is fine with me @YarShev 😄. I am always forgetting the license headers. |
What do these changes do?
Adds extensions capability to Modin.
flake8 modin/ asv_bench/benchmarks scripts/doc_checker.pyblack --check modin/ asv_bench/benchmarks scripts/doc_checker.pygit commit -sdocs/development/architecture.rstis up-to-date