8000 [SPRINT] Functions with *args can now have explicit keyword args · Issue #17345 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content
8000

[SPRINT] Functions with *args can now have explicit keyword args #17345

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
Closed
jnothman opened this issue May 25, 2020 · 13 comments · Fixed by #17623
Closed

[SPRINT] Functions with *args can now have explicit keyword args #17345

jnothman opened this issue May 25, 2020 · 13 comments · Fixed by #17623
Labels
Easy Well-defined and straightforward way to resolve good first issue Easy with clear instructions to resolve help wanted Sprint

Comments

@jnothman
Copy link
Member
jnothman commented May 25, 2020 •
edited by thomasjpfan
Loading

Please do not address this issue unless you're part of the sprint (June 6th 2020).

Functions like resample have a signature like def resample(*arrays, **options) because of a limitation of python 2, where named keyword arguments could not be used with variable number of positional argument. We now should be using explicit keyword arguments where possible.
@jnothman jnothman added Easy Well-defined and straightforward way to resolve Sprint good first issue Easy with clear instructions to resolve help wanted labels May 25, 2020
@jnothman jnothman mentioned this issue May 25, 2020
MNT Replace kwargs by named args for resample #17324
Merged
@jnothman
Copy link
Member Author

It turns out this specific case is fixed by #17324. Are there others?

@thomasjpfan
Copy link
Member
thomasjpfan commented May 25, 2020 •
edited by rth
Loading

Other others are:

  • make_pipeline
  • make_union
  • shuffle
  • make_column_transformer

@JosephTLucas
Copy link
Contributor

Can I work on this for the June 6th Sprint? I can do all three or just take one if other participants want to work on the others. If I take one, I'll take shuffle. It sounds like I would add random_state, and n_samples as keyword args.

If this is available, just let me know whether I should only do shuffle or can take all three.

@thomasjpfan
Copy link
Member

@JosephTLucas Let's do shuffle first. It would make it easier to review and it would be a good way to get started.

@loldja
Copy link
Contributor
loldja commented Jun 6, 2020

Take make_pipeline

@jaketae
Copy link
Contributor
jaketae commented Jun 6, 2020 •
edited
Loading

Take make_union

CC: @cemeiqra

@asubramaniyan
Copy link
Contributor

Take shuffle

@thomasjpfan
Copy link
Member

all of the issues are taken here for now.

(Although there may be more that I have not found)

@ab-anssi
Copy link
ab-anssi commented Jun 6, 2020

We are already working on shuffle with @JosephTLucas .

@JosephTLucas JosephTLucas mentioned this issue Jun 6, 2020
Merged
@loldja loldja mentioned this issue Jun 6, 2020
Merged
@alfaro96
Copy link
Member
alfaro96 commented Jun 16, 2020 •
edited
Loading

I think that all the issues have been already solved and merged (#17472, #17474, #17477).

Close?

@thomasjpfan
Copy link
Member

There was one more than I missed: make_column_transformer. (Added to the list)

@jaketae
Copy link
Contributor
jaketae commented Jun 16, 2020

I'll take a jab at make_column_transformer, unless it's already been worked out.

@thomasjpfan
Copy link
Member

Yup you can work on this.

@jaketae jaketae mentioned this issue Jun 17, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Easy Well-defined and straightforward way to resolve good first issue Easy with clear instructions to resolve help wanted Sprint
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Replace *kwargs with named arguments in make_column_transformer
8 participants
@jnothman @thomasjpfan @loldja @JosephTLucas @ab-anssi @jaketae @alfaro96 @asubramaniyan
0