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

Add sample_weight parameter in TLCenter, TLStretch and TLRotate #273

Merged
merged 12 commits into from Jan 16, 2024

Conversation

apmellot
Copy link
Contributor

@apmellot apmellot commented Dec 6, 2023

@sylvchev @agramfort
Following the discussion we had. I added a sample_weight parameter in .fit and .fit_transform in the 3 transfer learning classes. I tested it, and it runs properly.

Copy link

@antoinecollas antoinecollas 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 !

Copy link
Member

@qbarthelemy qbarthelemy left a comment

Choose a reason for hiding this comment

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

Thx @apmellot for this contribution!
Can you complete tests with sample weights different from None?

Copy link
Member

@agramfort agramfort 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 but it needs tests

@apmellot
Copy link
Contributor Author

apmellot commented Dec 7, 2023

The tests now include sample_weight different from None.

@qbarthelemy
Copy link
Member

@plcrodrigues
Copy link
Member

Hi there, thanks @apmellot for your help! I'm accepting this PR and would like to point out that sklearn now has its metadata routing which could be of interest to us in the near future.

@plcrodrigues plcrodrigues merged commit b9148ec into pyRiemann:master Jan 16, 2024
10 checks passed
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.

None yet

5 participants