8000 ENH Removes validation from __init__ for SGDOneClassSVM by iofall · Pull Request #21944 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

Comments

ENH Removes validation from __init__ for SGDOneClassSVM#21944

Closed
iofall wants to merge 5 commits intoscikit-learn:mainfrom
iofall:Issue-21406-SGDOneClassSVM
Closed

ENH Removes validation from __init__ for SGDOneClassSVM#21944
iofall wants to merge 5 commits intoscikit-learn:mainfrom
iofall:Issue-21406-SGDOneClassSVM

Conversation

@iofall
Copy link
Contributor
@iofall iofall commented Dec 10, 2021 •
edited
Loading

Addresses #21406

What does this implement/fix? Explain your changes.

  1. Changed alpha to None in __init__
  2. Previously alpha was initialized as self.nu / 2 in __init__ which was performed before validation of nu. This caused type errors in the __init__ method.
  3. Now alpha is initialized as None and modified after validation in its respective fit() & partial_fit() methods

cc: @arisayosh

@iofall @arisayosh
Remove validation from __init__ for SGDOneClassSVM
ccc861c 
Co-authored-by: iofall <50991099+iofall@users.noreply.github.com>
Co-authored-by: arisayosh <15692997+arisayosh@users.noreply.github.com>
@iofall iofall changed the title ENH Removes validation from init for SGDOneClassSVM ENH Removes validation from __init__ for SGDOneClassSVM Dec 10, 2021
iofall and others added 4 commits December 11, 2021 22:47
@iofall @arisayosh
Resolve merge conflicts
b4a782a 
Co-authored-by: arisayosh <15692997+arisayosh@users.noreply.github.com>
Co-authored-by: iofall <50991099+iofall@users.noreply.github.com>
@iofall
Merge branch 'main' into Issue-21406-SGDOneClassSVM
77998bb
@iofall
Merge branch 'Issue-21406-SGDOneClassSVM' of github.com:iofall/scikit…
afd592b 
…-learn into Issue-21406-SGDOneClassSVM
@iofall @arisayosh
Add Changelog
321b791 
Co-authored-by: arisayosh <15692997+arisayosh@users.noreply.github.com>
Co-authored-by: iofall <50991099+iofall@users.noreply.github.com>
@arisayosh arisayosh mentioned this pull request Dec 11, 2021
Closed
@betatim
Copy link
Member
betatim commented Sep 13, 2022 •
edited
Loading

Hi 👋

are you still working on this PR or could I take it over?

After rebasing this and resolving conflicts I think the only thing left to do is to remove alpha from __init__, all the other changes seem to have already been applied in other PRs. I think we don't need to pass alpha=None in __init__, instead we can not pass it.

@iofall
Copy link
Contributor Author
iofall commented Sep 13, 2022

Sure @betatim go ahead. We were just waiting for a review, but it has been a long time. I don't quite remember why we did the alpha = None, but looking at the recent changes, just removing that will work.

@betatim betatim mentioned this pull request Sep 14, 2022
Merged
@ogrisel
Copy link
Member
ogrisel commented Sep 15, 2022

Closing in favor of #24433.

@ogrisel ogrisel closed this Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

module:linear_model Waiting for Reviewer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@iofall @betatim @ogrisel @cmarmo
0