-
-
Notifications
You must be signed in to change notification settings - Fork 251
Add support for callable group_by #571
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
base: dev
Are you sure you want to change the base?
Conversation
6349447
to
eec4a54
Compare
@echan5 Thanks for the PR. What tests were failing and what is your environment? Tests are not failing on our CI so it must be your local env setup. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #571 +/- ##
=======================================
Coverage 96.07% 96.08%
=======================================
Files 16 16
Lines 4463 4465 +2
=======================================
+ Hits 4288 4290 +2
Misses 175 175 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
You're right - it was my setup. It didn't originally occur to me how the dependencies of this repo are structured and I had just run
But I just examined more closely and realized I should re-install the environment with Please let me know if anything else needs addressing - thank you! |
This reverts commit eec4a54.
Hi, just discovered this package and it's the functionality I'm looking for! However, I have one minor proposal that would make it work better for my use case with a complex nested data structure, and that is to allow the group_by param to be a callable so that I can access the more nested layers.
I'm also fixing some issues that I ran into when I ran:
python3 -m deepdiff.diff
for the docstring examples,pytest
due to missing dependenciesNote that I did run
pytest
and there were some failures but I think they were pre-existing as they also failed on the master branch, but without being familiar with the rest of the repo, please let me know if there might be any issues with this update!I'm also not sure what the process is from here - this PR is merging into the dev branch.