Skip to content

Conversation

@bodevone
Copy link
Contributor

@bodevone bodevone commented Jul 24, 2022

Pull Request check-list

Please make sure to review and check all of these items:

  • Does $ tox pass with this change (including linting)?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?
  • Was the change added to CHANGES file?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Description of change

Add TDIGEST.TRIMMED_MEAN command
Closes #2196

@bodevone
Copy link
Contributor Author

@dvora-h I am having problem with running test on correct version of BloomFilter (test fails to find command of TDIGEST.TRIMMED_MEAN)
Am I doing something wrong?

@dvora-h
Copy link
Collaborator

dvora-h commented Jul 27, 2022

@bodevone You're right, the issue is in the docker, I'll trigger the CI again after it will fix

@dvora-h
Copy link
Collaborator

dvora-h commented Aug 2, 2022

@bodevone redismod docker fixed, now it should work.

@dvora-h
Copy link
Collaborator

dvora-h commented Aug 2, 2022

@bodevone You need to add this command to MODULE_CALLBACKS (in init.py) to convert the response to float.
Let me know if you can do it or you want me to take it.

@dvora-h dvora-h added the feature New feature label Aug 4, 2022
@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2022

Codecov Report

Merging #2300 (b8737d6) into master (ab5272c) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@ Coverage Diff @@ ## master #2300 +/- ## ========================================== + Coverage 92.16% 92.18% +0.02%  ========================================== Files 111 111 Lines 28702 28713 +11 ========================================== + Hits 26454 26470 +16  + Misses 2248 2243 -5 
Impacted Files Coverage Δ
redis/commands/bf/__init__.py 100.00% <ø> (ø)
redis/commands/bf/commands.py 98.37% <100.00%> (+0.02%) ⬆️
tests/test_bloom.py 99.60% <100.00%> (+0.01%) ⬆️
tests/test_cluster.py 96.96% <0.00%> (+0.11%) ⬆️
tests/test_asyncio/test_pubsub.py 99.49% <0.00%> (+0.16%) ⬆️
tests/test_asyncio/test_search.py 98.71% <0.00%> (+0.32%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@dvora-h dvora-h merged commit 34f07a7 into redis:master Aug 4, 2022
@dvora-h
Copy link
Collaborator

dvora-h commented Aug 4, 2022

I hope it's okay that I ended up doing it myself because we wanted to include it in the next version

@bodevone
Copy link
Contributor Author

bodevone commented Aug 8, 2022

@dvora-h I am sorry for late response and yes it's totally okay

dvora-h added a commit that referenced this pull request Nov 21, 2022
* Add tdigest trimmed mean command with test * Add skip version for test * add to module callbacks Co-authored-by: Alibi Shalgymbay <a.shalgymbay@mycar.kz> Co-authored-by: dvora-h <67596500+dvora-h@users.noreply.github.com> Co-authored-by: dvora-h <dvora.heller@redis.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature

3 participants