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

Change **old_kwargs magic to explicit parameters #595

Merged
merged 3 commits into from Jan 25, 2020
Merged

Change **old_kwargs magic to explicit parameters #595

merged 3 commits into from Jan 25, 2020

Conversation

aleju
Copy link
Owner

@aleju aleju commented Jan 25, 2020

The parameter names of class Augmenter were recently changed
and the old names were caught by **old_kwargs. This was
convenient, but is changed in this patch to be explicitly
random_state="deprecated", deterministic="deprecated".
That is because the **old_kwargs magic has the disadvantage
of not necessarily being fully understood by IDEs. These then
cannot detect (and warn) about invalid parameters anymore.
Without the magic parameter they can warn about these,
hence it is the preferred choice here.

The parameter names of class `Augmenter` were recently changed
and the old names were caught by `**old_kwargs`. This was
convenient, but is changed in this patch to be explicitly
`random_state="deprecated", deterministic="deprecated"`.
That is because the `**old_kwargs` magic has the disadvantage
of not necessarily being fully understood by IDEs. These then
cannot detect (and warn) about invalid parameters anymore.
Without the magic parameter they can warn about these,
hence it is the preferred choice here.
@codecov-io
Copy link

codecov-io commented Jan 25, 2020

Codecov Report

Merging #595 into master will increase coverage by 0.01%.
The diff coverage is 95.35%.

@@            Coverage Diff             @@
##           master     #595      +/-   ##
==========================================
+ Coverage   96.32%   96.33%   +0.01%     
==========================================
  Files          41       41              
  Lines       14671    14668       -3     
==========================================
- Hits        14131    14130       -1     
+ Misses        540      538       -2

@aleju aleju merged commit 0db2ef8 into master Jan 25, 2020
@aleju aleju deleted the old_kwargs branch January 25, 2020 19:11
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

2 participants