Skip to content

chore: remove pop_with_default() function#532

Merged
mjs merged 1 commit intomjs:masterfrom
JohnVillalovos:jlvillal/simplify
Aug 27, 2023
Merged

chore: remove pop_with_default() function#532
mjs merged 1 commit intomjs:masterfrom
JohnVillalovos:jlvillal/simplify

Conversation

@JohnVillalovos
Copy link
Contributor

In current Pythons we can use dict.pop(key, default)

Also converted some **kwargs to two named arguments with defaults.

In current Pythons we can use `dict.pop(key, default)`

Also converted some `**kwargs` to two named arguments with defaults.
@axoroll7
Copy link
Contributor

axoroll7 commented Aug 27, 2023

Which current Pythons ?
Edit : Works with 3.6

@JohnVillalovos
Copy link
Contributor Author

Which current Pythons ? Edit : Works with 3.6

All the ones that are supported by the current version of imapclient

@axoroll7
Copy link
Contributor

Which current Pythons ? Edit : Works with 3.6

All the ones that are supported by the current version of imapclient

Ok thanks, I should have known

@JohnVillalovos
Copy link
Contributor Author

As a bit of trivia. It was added in Python 2.3.

https://docs.python.org/2.7/library/stdtypes.html#mapping-types-dict

Copy link
Owner

@mjs mjs left a comment

Choose a reason for hiding this comment

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

A nice little clean up. Thanks.

Comment on lines -1724 to -1727
def _command_and_check(self, command, *args, **kwargs):
unpack = pop_with_default(kwargs, "unpack", False)
uid = pop_with_default(kwargs, "uid", False)
assert not kwargs, "unexpected keyword args: " + ", ".join(kwargs)
Copy link
Owner

Choose a reason for hiding this comment

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

I think at some point kwargs was passed down to other calls but not any more so what you've done makes much more sense.

@mjs mjs merged commit 7465000 into mjs:master Aug 27, 2023
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.

3 participants