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

Dev/switch org api #541

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Dev/switch org api #541

wants to merge 13 commits into from

Conversation

vaimdev
Copy link
Contributor

@vaimdev vaimdev commented Jan 22, 2024

Changes to work with nexus' dev/switch_org_api.

Reviewed by Koa in progress.

setup.py Outdated
@@ -42,7 +42,7 @@ def unique_flatten_dict(d):
}

base_extras_heavy = {
'umap-learn': ['umap-learn', 'dirty-cat==0.2.0', 'scikit-learn==1.2.2'],
'umap-learn': ['umap-learn', 'dirty-cat==0.2.0', 'scikit-learn==1.3.2'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not edit dependencies without a specific clear reason

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(pydata GPU dependencies are fragile and require substantial testing for changes )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lmeyerov , checked with my colleague, he pinned this dependencies because there is an exception thrown with latest scikit-learn

TypeError: _iter() missing 3 required positional arguments: 'column_as_labels', 'skip_drop', and 'skip_empty_columns'

pinning to version 1.3.2 can prevent this error.
The error is coming from dirty_cat SuperVectorizer that was using scikit-learn. But SuperVectorizer is depreciated and replaced with TableVectorizer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already pinned, not latest

Copy link
Contributor

@lmeyerov lmeyerov Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove from this PR either way, if worthwhile, it should go in its own PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I'll remove it.
it will cause the test script failure, therefore, we pin it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we don't pin, test script will fail

Copy link
Contributor

@lmeyerov lmeyerov Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is about tests, we can patch test (non-prod) deps?

Working through a prod dep here would take more work to chase down, eg, GPU RAPIDS alignment

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From discussion:

  • we can revert to original of scikit > 1.0...
  • ... and in testai cfg, do a <,> to skip the problematic recent scikits (I forgot exact syntax):

setup.py Outdated
@@ -27,7 +27,8 @@ def unique_flatten_dict(d):
'docs': ['sphinx==3.4.3', 'docutils==0.16', 'sphinx_autodoc_typehints==1.11.1', 'sphinx-rtd-theme==0.5.1', 'Jinja2<3.1'],
'test': ['flake8>=5.0', 'mock', 'mypy', 'pytest'] + stubs,
'testai': [
'numba>=0.57.1' # https://github.com/numba/numba/issues/8615
'numba>=0.57.1', # https://github.com/numba/numba/issues/8615
'scikit-learn<=1.3.2'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also > the known bad version?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scikit-learn<=1.3.2,>1.4.0 give:
ERROR: No matching distribution found for scikit-learn<=1.3.2,>1.4.0
I use not equal instead

@lmeyerov
Copy link
Contributor

#548 is landing, should solve CI fail

where are we on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants