8000 [arm64] MNT skip docstring check in tutorial for ARM64 by glemaitre · Pull Request #18728 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

[arm64] MNT skip docstring check in tutorial for ARM64 #18728

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 2, 2020
Merged

[arm64] MNT skip docstring check in tutorial for ARM64 #18728

merged 3 commits into from
Nov 2, 2020

Conversation

glemaitre
Copy link
Member

Check doctest failure in arm64

ogrisel
ogrisel approved these changes Nov 2, 2020
View reviewed changes
Copy link
Member
@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM.

Alternatively we could skip all the doctests on the ARM64 cron job as we don't care about small platform specific variations in doctest displays.

NicolasHug
NicolasHug approved these changes Nov 2, 2020
View reviewed changes
Copy link
Member
@NicolasHug NicolasHug left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This doesn't just skip for ARM right?

Alternatively we could skip all the doctests on the ARM64 cron job as we don't care about small platform specific variations in doctest displays.

+1 for that

LGTM anyway

< 8000 div data-view-component="true" class="TimelineItem-body my-0">
@ogrisel
Copy link
Member
ogrisel commented Nov 2, 2020

@glemaitre we have a similar failure in sgd.rst now:

=================================== FAILURES ===================================

______________________________ [doctest] sgd.rst _______________________________

[gw3] linux -- Python 3.7.8 /home/travis/miniconda/envs/testenv/bin/python3.7

124 parameters if an example violates the margin constraint, which makes

125 training very efficient and may result in sparser models (i.e. with more zero

126 coefficents), even when L2 penalty is used.

127 

128 Using ``loss="log"`` or ``loss="modified_huber"`` enables the

129 ``predict_proba`` method, which gives a vector of probability estimates

130 :math:`P(y|x)` per sample :math:`x`::

131 

132     >>> clf = SGDClassifier(loss="log", max_iter=5).fit(X, y)

133     >>> clf.predict_proba([[1., 1.]])

Expected:

    array([[0.00..., 0.99...]])

Got:

    array([[4.97248476e-07, 9.99999503e-01]])

so instead of puting skip markers everywhere we should rather skip the doctests for the ARM64 CI instead.

@glemaitre
Copy link
Member Author

so instead of puting skip markers everywhere we should rather skip the doctests for the ARM64 CI instead.

agreed

@glemaitre
Copy link
Member Author

So this PR makes 2 changes

  • remove the --doctest-modules from the setup.cfg when the tag [arm64] is given. I could not find how to overwrite the options with pytest. You have -o but here you don't want to set a value, you want to remove the flag.
  • do not run the documentation test for [arm64]
8000

ogrisel
ogrisel approved these changes Nov 2, 2020
View reviewed changes
Copy link
Member
@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM!

Let's merge, we can always tweak later.

@ogrisel ogrisel merged commit 4876688 into scikit-learn:master Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogrisel ogrisel ogrisel approved these changes

@NicolasHug NicolasHug NicolasHug approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@glemaitre @ogrisel @NicolasHug
0