Skip to content
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

Fix CI, including mypy "incompatible return type" error in modin/__init__.py:64:12 #7419

Closed
sfc-gh-mvashishtha opened this issue Jan 11, 2025 · 6 comments · Fixed by #7420
Closed
Assignees
Labels

Comments

@sfc-gh-mvashishtha
Copy link
Contributor

see CI failure here: https://github.com/modin-project/modin/actions/runs/12682127213/job/35347029009

there may be other errors as well

@sfc-gh-mvashishtha sfc-gh-mvashishtha self-assigned this Jan 11, 2025
sfc-gh-mvashishtha added a commit to sfc-gh-mvashishtha/modin that referenced this issue Jan 11, 2025
Signed-off-by: sfc-gh-mvashishtha <[email protected]>
@sfc-gh-mvashishtha
Copy link
Contributor Author

Switching miniforge-variant from Mambaforge to Miniforge3 seemed to fix the mamba-env issue: conda-incubator/setup-miniconda#383

@sfc-gh-mvashishtha
Copy link
Contributor Author

sfc-gh-mvashishtha commented Jan 13, 2025

A few other things came up in CI on the draft PR:

I am considering removing testing for unidist and MPI.

@anmyachev
Copy link
Collaborator

Hi @sfc-gh-mvashishtha!

some windows tests fail and seem to say that CONDA_PKGS_DIR is empty e.g. here: https://github.com/modin-project/modin/actions/runs/12753981920/job/35546823329?pr=7420

This was only needed to speed up the tests, you can safely delete it. Then, if necessary, we can see what has changed and how to return it.

I am considering removing testing for unidist and MPI.

@YarShev do you know what's wrong with unidist in CI?

@sfc-gh-mvashishtha
Copy link
Contributor Author

sfc-gh-mvashishtha commented Jan 15, 2025

@anmyachev because unidist is not being maintained, I'm going to remove CI tests for unidist. cc @devin-petersohn

For the record, we did try two different MPI version restrictions, but they didn't work: #7420 (comment)

@anmyachev
Copy link
Collaborator

anmyachev commented Jan 15, 2025

@anmyachev because unidist is not being maintained, I'm going to remove CI tests for unidist. cc @devin-petersohn

For the record, we did try two different MPI version restrictions, but they didn't work: #7420 (comment)

The main tests still work, only one with installation via APT does not work. #7420 (comment)

sfc-gh-mvashishtha added a commit that referenced this issue Jan 16, 2025
- Use Miniforge3 instead of Mambaforge in conda-incubator/setup-miniconda action, per conda-incubator/setup-miniconda#383
- Skip test that tries to use Modin with unidist installed via APT. See #7421 for details.
- Remove manual conda packaging caching, an optimization which was added to speed up CI but now causes an error complaining that `CONDA_PKGS_DIR` is empty.
- Fix some mypy errors in modin/__init__.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants