-
-
Notifications
You must be signed in to change notification settings - Fork 252
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
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.
✅ Project coverage is 96.08%. Comparing base (0978fb8) to head (9a673fe).
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.
📢 Have feedback on the report? Share it here.
🚀 New features to boost your workflow:
- ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
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 uv sync --extra test (using python 3.12) before running uv run pytest tests (and adding any missing imports that came up like uv pip install numpy pytz click uuid6 pydantic and had gotten various assertion errors like the below (the list was actually much longer), which at first glance seemed to be discrepancies with spacing in strings:
FAILED tests/test_delta.py::TestBasicsOfDelta::test_delta_repr - assert '<Delta: {"iterable_item_added": {"root[2]": 3, "root[3]": 5}}>' in {'<Delta: {"iterable_item_added":{"root[2]":3,"root[3]":5}}>...
FAILED tests/test_summarize.py::TestSummarize::test_list_summary - assert '[100,101,102,103,10,"..."]' == '[100, 101, 1...3, 10, "..."]'
But I just examined more closely and realized I should re-install the environment with uv sync --all-extras and all the tests pass now. I'll also revert the changes I had put into pyproject.toml/uv.lock thinking they were to solve these dependency issues.
Please let me know if anything else needs addressing - thank you!
This reverts commit eec4a54.
Hi @echan5 Thanks for the PR. Reviewing shortly.
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.difffor the docstring examples,pytestdue to missing dependenciesNote that I did run
pytestand 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.