Skip to content

DOC details on the use of xfail_checks#16968

Merged
rth merged 2 commits intoscikit-learn:masterfrom
NicolasHug:xfaildocs
Apr 20, 2020
Merged

DOC details on the use of xfail_checks#16968
rth merged 2 commits intoscikit-learn:masterfrom
NicolasHug:xfaildocs

Conversation

@NicolasHug
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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