Skip to content

DOC details on the use of xfail_checks #16968

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

Merged
merged 2 commits into from
Apr 20, 2020
Merged

Conversation

NicolasHug
Copy link
Member

Basically addresses #16890 (comment)

CC @rth @thomasjpfan

Might need an update depending on the outcome of #16963

Comment on lines 526 to 529
Of all tags, the usage of this one is highly subject to change because we
are trying to make it more flexible in the future. Don't use this unless
you have a *very good* reason, and be prepared for breaking changes in
the future.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Of all tags, the usage of this one is highly subject to change because we
are trying to make it more flexible in the future. Don't use this unless
you have a *very good* reason, and be prepared for breaking changes in
the future.
Warning: the API of this tag is likely to change in v0.24 in a backward compatible way.

I don't agree with recommending not to use it. Yes, the API may change in the future (but we are not even 100% sure), stating it and letting users decide if they are willing to make the migration for v0.24 is enough IMO.

Copy link
Member

Choose a reason for hiding this comment

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

OK I see "Don't use this unless you have a very good reason." was there before, we can leave it then.

Copy link
Member Author

Choose a reason for hiding this comment

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

Though yeah the "don't use this" is more about "be careful about diverting from strict compatibility". I'll reorder

Copy link
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

Thanks @NicolasHug !

@rth rth changed the title [MRG] DOC details on the use of xfail_checks DOC details on the use of xfail_checks Apr 20, 2020
@rth rth merged commit 6973096 into scikit-learn:master Apr 20, 2020
gio8tisu pushed a commit to gio8tisu/scikit-learn that referenced this pull request May 15, 2020
viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020
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.

2 participants