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 subdict function #150

Merged
merged 3 commits into from
Mar 2, 2018
Merged

Add subdict function #150

merged 3 commits into from
Mar 2, 2018

Conversation

acatton
Copy link
Contributor

@acatton acatton commented Feb 4, 2018

This adds an utility function to compute a "sub-dictionnaries" to dictutils.py.

I've worked on a project with @mdomke and he was adding a very similar function to all of his projects utility module. At first I thought "whatever dude..." But then I realized that it actually is a very useful function, and I ended up adding the same function to all my projects utils.py.

I think this would be a very useful addition to boltons

Disclaimer: I wrote this function from scratch, just copying the function signature/API from @mdomke's original function. I made sure to use a different python style than him, so that no copyright infringement could be claimed. (Even though, I think @mdomke would not mind if I would've just had copy/pasted his original code.)

@acatton acatton force-pushed the subdict branch 2 times, most recently from 9a75073 to faaa8b4 Compare February 4, 2018 20:29
The concept was stolen from @mdomke
@mdomke
Copy link

mdomke commented Feb 4, 2018

You're right @acatton, I would be cool with you reusing the original code… I'm glad that you came up with the idea adding this function to boltons.

@mahmoud
Copy link
Owner

mahmoud commented Feb 4, 2018

I agree with you both, this looks good, thanks! Would you be ok if I renamed restrict_to/omit to include/exclude? I think those names might be more intuitive, but I'm open to others, too.

@mdomke
Copy link

mdomke commented Feb 4, 2018

I think semantically it is okay to replace omit with exclude. On the other hand I would say that restrict_to and include are not the same. When having set theory in mind restrict_to translates for me as "take the subset with these keys", whereas include would intuitively translate into some kind of union operation (even though it doesn't make sense from a functionality point of view). But that's just my opinion…

@mahmoud
Copy link
Owner

mahmoud commented Feb 5, 2018

Hmm, true. How about keep and skip (or drop)?

@acatton
Copy link
Contributor Author

acatton commented Feb 5, 2018

I personally like keep and drop. I updated the code.

@mahmoud thanks for the quick reply 😉 .

@mahmoud mahmoud merged commit 78c42d8 into mahmoud:master Mar 2, 2018
@mahmoud
Copy link
Owner

mahmoud commented Mar 2, 2018

Freshly released in 18.0.0, thank you again for your contribution!

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