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

Sklearn update #1

Merged
merged 30 commits into from
Feb 22, 2024
Merged

Sklearn update #1

merged 30 commits into from
Feb 22, 2024

Conversation

cae67
Copy link
Collaborator

@cae67 cae67 commented Feb 19, 2024

OVERVIEW:
Updated polyssifier to enable it to install via pip and run in Python 3 while maintaining Python 2.7 compatibility.

Note that due to differences in Windows and Linux multiprocessing an additional line of code must be added prior to calling polyssifier in Windows:

if name == 'main':
report = polyssifier.poly(data,labels)

TESTING:
Passed tests on Linux Python 3.7-3.12
Passed tests on Windows Python 3.7-3.12
Passed tests on Windows Python 2.7
Passed most tests on Linux Python 2.7

MODIFICATIONS:
Setup.py – changed ‘sklearn’ in install_requires to ‘scikit-learn’
Setup.py – added ‘joblib’ to install_requires to maintain Python 2.7 compatibility
Setup.py – added Python versions 3.7-3.12
Polyssifier.py – line 58 changed because fstring doesn’t work in Python 2
Poly_utils.py – lines 128 and 135 changed because max_features=’auto’ only works in scikit-learn 0.x. Switched to ‘sqrt’, which has identical outcome to ‘auto’ in scikit-learn 0.x but also works in scikit-learn 1.x.
Test_classification.py – lines 39 and 48, changed iteritems() to items() because iteritems() was deprecated in pandas 2.0
Test_multiclass.py – line 29, changed iteritems() to items() because iteritems() was deprecated in pandas 2.0
Test_regression.py – line 28, changed iteritems() to items() because iteritems() was deprecated in pandas 2.0

@coveralls
Copy link

Pull Request Test Coverage Report for Build 7965984878

Details

  • 0 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.514%

Totals Coverage Status
Change from base Build 7910775236: 0.0%
Covered Lines: 482
Relevant Lines: 521

💛 - Coveralls

Copy link
Collaborator

@bbradt bbradt left a comment

Choose a reason for hiding this comment

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

All changes look reasonable to me. If the library works per the current version of the README, I would recommend merging.

Copy link

@sergeyplis sergeyplis left a comment

Choose a reason for hiding this comment

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

just one comment about auto

polyssifier/poly_utils.py Show resolved Hide resolved
Copy link

@sergeyplis sergeyplis left a comment

Choose a reason for hiding this comment

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

all's clear

@sergeyplis sergeyplis merged commit 72f8008 into master Feb 22, 2024
1 check passed
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