Skip to content

[DOC] solves several small raised issues #266

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 5 commits into from
Nov 21, 2019
Merged

[DOC] solves several small raised issues #266

merged 5 commits into from
Nov 21, 2019

Conversation

RobinVogel
Copy link
Contributor

Solves #258, #261, #256.

Copy link
Contributor

@perimosocordiae perimosocordiae left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -273,6 +273,7 @@ class ITML_Supervised(_BaseITML, TransformerMixin):
be removed in 0.6.0.
num_constraints: int, optional
number of constraints to generate
(`20 * num_classes**2` constraints by default)
Copy link
Contributor

Choose a reason for hiding this comment

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

We may need to use double-backticks here to prevent the docs from interpreting this as a link target. Can you build the HTML docs with these changes to verify?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried it locally and I see it works as expected, also there is a itml_supervised.fit(X, y, bounds=...) that does not uses double backticks in the code a few lines below.

Should I make a pull request for the documentation that I generated as well ?
The commits that update the documentation look a bit heavy for minor modifications and include some system-specific results.

Copy link
Member

@bellet bellet left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Let's not do a PR for updating doc. One of us can push directly to the doc branch

@perimosocordiae perimosocordiae merged commit 710379e into scikit-learn-contrib:master Nov 21, 2019
@perimosocordiae
Copy link
Contributor

Merged. Thanks!

@bellet
Copy link
Member

bellet commented Nov 21, 2019

@RobinVogel: for next PRs, you can use keywords which will automatically close the issues when the PR is merged, see
https://help.github.com/en/github/managing-your-work-on-github/closing-issues-using-keywords

@RobinVogel RobinVogel deleted the documentation branch November 28, 2019 15:42
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.

3 participants