Bloom Filter by isidroas · Pull Request #8615 · TheAlgorithms/Python

isidroas · 2023-04-06T12:13:59Z

Describe your change:

Add an algorithm?
Fix a bug or typo in an existing algorithm?
Documentation change?

Checklist:

algorithms-keeper

Click here to look at the relevant links ⬇️

🔗 Relevant Links
Repository:
Contributing guidelines
Project Euler solution guidelines
Python:
Formatted string literals (f-strings)
Type hints
doctest
unittest
pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper review to trigger the checks for only added pull request files
@algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.
NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

algorithms-keeper

Click here to look at the relevant links ⬇️

🔗 Relevant Links
Repository:
Contributing guidelines
Project Euler solution guidelines
Python:
Formatted string literals (f-strings)
Type hints
doctest
unittest
pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper review to trigger the checks for only added pull request files
@algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.
NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

algorithms-keeper

Click here to look at the relevant links ⬇️

🔗 Relevant Links
Repository:
Contributing guidelines
Project Euler solution guidelines
Python:
Formatted string literals (f-strings)
Type hints
doctest
unittest
pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper review to trigger the checks for only added pull request files
@algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.
NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

CaedenPH

As all our algorithms are tested via doctest, a docstring testing module, test functions using assert will not fail the tests. Please convert your tests to doctest

CaedenPH · 2023-04-06T20:39:06Z

+ assert b.exists("Titanic")
+ assert b.exists("Avatar")
+
+ assert b.exists("The Goodfather") in (True, False)
+ assert b.exists("Interstellar") in (True, False)
+ assert b.exists("Parasite") in (True, False)
+ assert b.exists("Pulp Fiction") in (True, False)


We use doctest to test our modules via a workflow

Moved to module docstring

CaedenPH · 2023-04-06T20:41:01Z

+ print(
+ f"""\
+[exists] value = {value}
+ hash = {self.format_bin(h)}
+ filter = {self.format_bin(self.bitstring)}
+ res = {res}
+"""
+ )


Suggested change
print(
f"""\
[exists] value= {value}
hash= {self.format_bin(h)}
filter= {self.format_bin(self.bitstring)}
res= {res}
"""
)
In the CONTRIBUTING it says
return all calculation results instead of printing or plotting them
I don't think these prints are necessary

moved to method and called in doctest

CaedenPH · 2023-04-06T20:41:10Z

+ print(
+ f"""\
+[add] value = {value}
+ hash = {self.format_bin(h)}
+ filter = {self.format_bin(self.bitstring)}
+"""
+ )


Suggested change
print(
f"""\
[add] value= {value}
hash= {self.format_bin(h)}
filter= {self.format_bin(self.bitstring)}
"""
)
Same here

CaedenPH · 2023-04-06T20:41:58Z

+ assert (
+ abs(estimated_error_rate - error_rate) <= 0.05
+ ) # 5% absolute margin calculated experiementally


Same as above, this should be converted to a doctest

In this case I removed the test. I found difficult to make tests with random values.

algorithms-keeper

Click here to look at the relevant links ⬇️

🔗 Relevant Links
Repository:
Contributing guidelines
Project Euler solution guidelines
Python:
Formatted string literals (f-strings)
Type hints
doctest
unittest
pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper review to trigger the checks for only added pull request files
@algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.
NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

cclauss · 2023-04-08T14:28:36Z

+Not added elements should return False ...
+>>> "The Goodfather" in bloom
+False
+>>> bloom.format_hash("The Goodfather")
+'00011000'
+>>> "Interstellar" in bloom
+False
+>>> bloom.format_hash("Interstellar")
+'00000011'
+>>> "Parasite" in bloom
+False
+>>> bloom.format_hash("Parasite")
+'00010010'
+>>> "Pulp Fiction" in bloom
+False
+>>> bloom.format_hash("Pulp Fiction")
+'10000100'


Should this be The Goodfather or The Godfather? https://www.imdb.com/title/tt0068646
Suggested change
NotaddedelementsshouldreturnFalse ...
>>>"The Goodfather"inbloom
False
>>>bloom.format_hash("The Goodfather")
'00011000'
>>>"Interstellar"inbloom
False
>>>bloom.format_hash("Interstellar")
'00000011'
>>>"Parasite"inbloom
False
>>>bloom.format_hash("Parasite")
'00010010'
>>>"Pulp Fiction"inbloom
False
>>>bloom.format_hash("Pulp Fiction")
'10000100'
NotaddedelementsshouldreturnFalse ...
>>>not_present_films= ("The Goodfather", "Interstellar", "Parasite", "Pulp Fiction")
>>> {film: bloom.format_hash(film) forfilminnot_present_films)}
{'The Goodfather': '00011000', 'Interstellar': '00000011', 'Parasite': '00010010': 'Pulp Fiction': '10000100'}
>>>any(filminbloomforfilminnot_present_films)
False

I had to reduce the tuple in order to avoid the following ruff error:
Also tried pretty-print, but it doesn't match

https://github.com/search?q=repo%3ATheAlgorithms%2FPython+NORMALIZE_WHITESPACE&type=code

Thanks for teaching me that.
I also had to divide in multiple lines the previous line:

Co-authored-by: Christian Clauss <cclauss@me.com>

for more information, see https://pre-commit.ci

This reverts commit 35fa5f5.

for more information, see https://pre-commit.ci

* Bloom filter with tests * has functions constant * fix type * isort * passing ruff * type hints * type hints * from fail to erro * captital leter * type hints requested by boot * descriptive name for m * more descriptibe arguments II * moved movies_test to doctest * commented doctest * removed test_probability * estimated error * added types * again hash_ * Update data_structures/hashing/bloom_filter.py Co-authored-by: Christian Clauss <cclauss@me.com> * from b to bloom * Update data_structures/hashing/bloom_filter.py Co-authored-by: Christian Clauss <cclauss@me.com> * Update data_structures/hashing/bloom_filter.py Co-authored-by: Christian Clauss <cclauss@me.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * syntax error in dict comprehension * from goodfather to godfather * removed Interestellar * forgot the last Godfather * Revert "removed Interestellar" This reverts commit 35fa5f5. * pretty dict * Apply suggestions from code review * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update bloom_filter.py --------- Co-authored-by: Christian Clauss <cclauss@me.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

algorithms-keeperBot added awaiting reviews This PR is ready to be reviewed require descriptive names This PR needs descriptive function and/or variable names require type hints https://docs.python.org/3/library/typing.html labels Apr 6, 2023

algorithms-keeperBot reviewed Apr 6, 2023
View reviewed changes

Comment threaddata_structures/hashing/bloom_filter.py Outdated
Comment threaddata_structures/hashing/bloom_filter.py Outdated

algorithms-keeperBot removed the require type hints https://docs.python.org/3/library/typing.html label Apr 6, 2023

algorithms-keeperBot reviewed Apr 6, 2023
View reviewed changes

Comment threaddata_structures/hashing/bloom_filter.py Outdated

algorithms-keeperBot removed the require descriptive names This PR needs descriptive function and/or variable names label Apr 6, 2023

CaedenPH requested changes Apr 6, 2023
View reviewed changes

algorithms-keeperBot added the require type hints https://docs.python.org/3/library/typing.html label Apr 7, 2023

algorithms-keeperBot reviewed Apr 7, 2023
View reviewed changes

Comment threaddata_structures/hashing/bloom_filter.py Outdated
Comment threaddata_structures/hashing/bloom_filter.py Outdated
Comment threaddata_structures/hashing/bloom_filter.py Outdated

algorithms-keeperBot removed the require type hints https://docs.python.org/3/library/typing.html label Apr 7, 2023

isidroas added 18 commits April 7, 2023 17:59

Bloom filter with tests
173ab0e

has functions constant
08bc970

fix type
0448109

isort
486dcbc

passing ruff
4111807

type hints
e6ce098

type hints
e4d39db

from fail to erro
7629686

captital leter
3926167

type hints requested by boot
280ffa0

descriptive name for m
5d460aa

more descriptibe arguments II
cc54095

moved movies_test to doctest
78d19fd

commented doctest
8b1bec0

removed test_probability
28e6691

estimated error
2fd7196

added types
314237d

again hash_
9b01472

from b to bloom
313c80c

algorithms-keeperBot removed the tests are failing Do not merge until tests pass label Apr 8, 2023

cclauss requested changes Apr 8, 2023
View reviewed changes

algorithms-keeperBot added awaiting changes A maintainer has requested changes to this PR and removed awaiting reviews This PR is ready to be reviewed labels Apr 8, 2023

Update data_structures/hashing/bloom_filter.py
18e0dde
Co-authored-by: Christian Clauss <cclauss@me.com>

algorithms-keeperBot added awaiting reviews This PR is ready to be reviewed and removed awaiting changes A maintainer has requested changes to this PR labels Apr 8, 2023

isidroasand others added 2 commits April 8, 2023 16:49

Update data_structures/hashing/bloom_filter.py
54041ff
Co-authored-by: Christian Clauss <cclauss@me.com>

[pre-commit.ci] auto fixes from pre-commit.com hooks
483a2a0
for more information, see https://pre-commit.ci

algorithms-keeperBot added the tests are failing Do not merge until tests pass label Apr 8, 2023

isidroas added 3 commits April 8, 2023 17:01

syntax error in dict comprehension
174ce08

from goodfather to godfather
00cc60e

removed Interestellar
35fa5f5

algorithms-keeperBot removed the tests are failing Do not merge until tests pass label Apr 8, 2023

isidroas added 3 commits April 8, 2023 17:33

forgot the last Godfather
5cd20ea

Revert "removed Interestellar"
7617143
This reverts commit 35fa5f5.

pretty dict
799171a

cclauss approved these changes Apr 8, 2023
View reviewed changes

algorithms-keeperBot removed the awaiting reviews This PR is ready to be reviewed label Apr 8, 2023

Apply suggestions from code review
1a71f4c

algorithms-keeperBot added the awaiting reviews This PR is ready to be reviewed label Apr 8, 2023

[pre-commit.ci] auto fixes from pre-commit.com hooks
4e0263f
for more information, see https://pre-commit.ci

algorithms-keeperBot added the tests are failing Do not merge until tests pass label Apr 8, 2023

Update bloom_filter.py
e746746

algorithms-keeperBot removed the tests are failing Do not merge until tests pass label Apr 8, 2023

cclauss merged commit 14bdd17 into TheAlgorithms:masterApr 8, 2023

algorithms-keeperBot removed the awaiting reviews This PR is ready to be reviewed label Apr 8, 2023

isidroas mentioned this pull request Jan 25, 2025
dh isidroas/Python#1
Closed
14 tasks

-print(
- f"""\
-[exists] value= {value}
-hash= {self.format_bin(h)}
-filter= {self.format_bin(self.bitstring)}
-res= {res}
-"""
- )

-NotaddedelementsshouldreturnFalse ...
->>>"The Goodfather"inbloom
-False
->>>bloom.format_hash("The Goodfather")
-'00011000'
->>>"Interstellar"inbloom
-False
->>>bloom.format_hash("Interstellar")
-'00000011'
->>>"Parasite"inbloom
-False
->>>bloom.format_hash("Parasite")
-'00010010'
->>>"Pulp Fiction"inbloom
-False
->>>bloom.format_hash("Pulp Fiction")
-'10000100'
+NotaddedelementsshouldreturnFalse ...
+>>>not_present_films= ("The Goodfather", "Interstellar", "Parasite", "Pulp Fiction")
+>>> {film: bloom.format_hash(film) forfilminnot_present_films)}
+{'The Goodfather': '00011000', 'Interstellar': '00000011', 'Parasite': '00010010': 'Pulp Fiction': '10000100'}
+>>>any(filminbloomforfilminnot_present_films)
+False

Uh oh!

Uh oh!

Conversation

isidroas commented Apr 6, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your change:

Checklist:

Uh oh!

algorithms-keeperBot left a comment

Choose a reason for hiding this comment

🔗 Relevant Links

Repository:

Python:

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper actions can be triggered by commenting on this PR:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

algorithms-keeperBot left a comment

Choose a reason for hiding this comment

🔗 Relevant Links

Repository:

Python:

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper actions can be triggered by commenting on this PR:

Uh oh!

Uh oh!

Uh oh!

algorithms-keeperBot left a comment

Choose a reason for hiding this comment

🔗 Relevant Links

Repository:

Python:

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper actions can be triggered by commenting on this PR:

Uh oh!

Uh oh!

CaedenPH left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

algorithms-keeperBot left a comment

Choose a reason for hiding this comment

🔗 Relevant Links

Repository:

Python:

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper actions can be triggered by commenting on this PR:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

isidroas commented Apr 6, 2023•
edited
Loading