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

Compatibility with Python 3.7.0b5 #11224

Closed
rth opened this issue Jun 9, 2018 · 7 comments · Fixed by #11256
Closed

Compatibility with Python 3.7.0b5 #11224

rth opened this issue Jun 9, 2018 · 7 comments · Fixed by #11256
Labels
Easy Well-defined and straightforward way to resolve good first issue Easy with clear instructions to resolve help wanted

Comments

@rth
Copy link
Member

rth commented Jun 9, 2018

When running the tests suite on Python 3.7.0b5, with

Dockerfile
FROM python:3.7-rc

RUN apt-get update && apt-get install -y build-essential git make

RUN apt-get install -y libopenblas-dev

RUN python --version

RUN pip3 --version

RUN pip3 install Cython nose pytest numpy
RUN pip3 install scipy

RUN git clone --depth 1 https://github.com/scikit-learn/scikit-learn.git && \
    cd scikit-learn && git checkout master


RUN cd scikit-learn && pip3 install -e .

RUN cd scikit-learn && pytest sklearn

most tests appears to pass, but there are two minor doctest failures,

________________ [doctest] sklearn.exceptions.FitFailedWarning _________________
106     Examples
107     --------
108     >>> from sklearn.model_selection import GridSearchCV
109     >>> from sklearn.svm import LinearSVC
110     >>> from sklearn.exceptions import FitFailedWarning
111     >>> import warnings
112     >>> warnings.simplefilter('always', FitFailedWarning)
113     >>> gs = GridSearchCV(LinearSVC(), {'C': [-1, -2]}, error_score=0, cv=2)
114     >>> X, y = [[1, 2], [3, 4], [5, 6], [7, 8]], [0, 0, 1, 1]
115     >>> with warnings.catch_warnings(record=True) as w:
Expected:
    FitFailedWarning('Estimator fit failed. The score on this train-test
    partition for these parameters will be set to 0.000000.
    Details: \nValueError: Penalty term must be positive; got (C=-2)\n',)
Got:
    FitFailedWarning('Estimator fit failed. The score on this train-test partition for these parameters will be set to 0.000000. Details: \nValueError: Penalty term must be positive; got (C=-2)\n')

/scikit-learn/sklearn/exceptions.py:115: DocTestFailure
_________________ [doctest] sklearn.exceptions.NotFittedError __________________
019 Exception class to raise if estimator is used before fitting.
020 
021     This class inherits from both ValueError and AttributeError to help with
022     exception handling and backward compatibility.
023 
024     Examples
025     --------
026     >>> from sklearn.svm import LinearSVC
027     >>> from sklearn.exceptions import NotFittedError
028     >>> try:
Expected:
    NotFittedError('This LinearSVC instance is not fitted yet',)
Got:
    NotFittedError('This LinearSVC instance is not fitted yet')

Also there is a DeprecationWarning when running import sklearn reported in #11121

@amueller
Copy link
Member

amueller commented Jun 9, 2018

That is a weird change in the standard exception __repr__. That's what this is, right?

@rth
Copy link
Member Author

rth commented Jun 9, 2018

Yes, seems due to https://bugs.python.org/issue30399 that was resolved - not sure why we still get this in 3.7.0b5...

@rth
Copy link
Member Author

rth commented Jun 9, 2018

No actually that issue / PR is where the behavior changed: in 3.7 it seems there is no trailing comma in Exceptions __repr__ anymore.

@amueller amueller added Easy Well-defined and straightforward way to resolve good first issue Easy with clear instructions to resolve help wanted labels Jun 9, 2018
@wenhaoz-fengcai
Copy link

Can I take this issue as my first contribution?

My idea of fixing this would be just removing the trailing comma in NotFittedError('This LinearSVC instance is not fitted yet',) and Details: \nValueError: Penalty term must be positive; got (C=-2)\n',).

@rth
Copy link
Member Author

rth commented Jun 13, 2018

Yes, thank you @jiaowoshabi !

@wenhaoz-fengcai
Copy link

@rth Hi, since Exceptions __rep__ in 3.6.5 still automatically adds the trailing comma at the end, test run failed on Python3.6.5 after simply removing the trailing comma.

How should I handle this backward compatibility if test run needs to succeed on 3.6.5?

@jnothman
Copy link
Member

jnothman commented Jun 14, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Easy Well-defined and straightforward way to resolve good first issue Easy with clear instructions to resolve help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants