Skip to content
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

VOTE propose vote for SLEP007 #59

Merged
merged 2 commits into from
Nov 29, 2021

Conversation

glemaitre
Copy link
Member

In this PR, I propose a vote for SLEP007.

As per our governance, the vote will last 1 month and will be announced shortly on the internal mailing list. We will seek a consensus during this month. If no consensus is found, the decision will be escalated to the Technical Committee (TC).

@agramfort
Copy link
Member

+1 That will be a great addition to sklearn !

@glemaitre
Copy link
Member Author

+1 as well

@TomDLT
Copy link
Member

TomDLT commented Oct 26, 2021

+1

2 similar comments
@adrinjalali
Copy link
Member

+1

@lorentzenchr
Copy link
Member

+1

@jjerphan
Copy link
Member

jjerphan commented Nov 1, 2021

Just to clarify: my approval for this PR stands for a +1 for the vote.

@thomasjpfan
Copy link
Member

+1 with minor adjustment to verbose_feature_names_out in ColumnTransformer: #60

@jnothman
Copy link
Member

jnothman commented Nov 6, 2021

The SLEP says nothing of the types of the feature names (str vs other) or the collection of them (list vs array). Should it? Otherwise, I'm happy with this.

(I have wondered, however, whether we give ourselves headaches by trying to ascribe names, rather than objects with structure and behaviour, to the representation of features. But certainly, we'd be giving ourselves more headaches if we wanted to attach such structured objects to DataFrames!)

@glemaitre
Copy link
Member Author

The SLEP says nothing of the types of the feature names (str vs other) or the collection of them (list vs array).

This is probably something that we should clarify by amending the current proposal as done there: #59 (comment)

@thomasjpfan
Copy link
Member

Opened #61 to add more details regarding the container, dtype, and strings.

@glemaitre
Copy link
Member Author

The vote has been open for a month and a consensus has been reached.
By our decision-making process, I changed the status of this SLEP to "Accepted".

We can provide post-history changes to give more details and improve the current proposal.

ping @adrinjalali @thomasjpfan did I miss anything in the process? I let you review and merge this PR if everything is OK.

@adrinjalali
Copy link
Member

I agree, and I think we've gone forward with the implementation and we seem to have a good consensus now.

@adrinjalali adrinjalali merged commit c7d1b3c into scikit-learn:master Nov 29, 2021
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.

8 participants