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

Make ipnetwork and ipaddress hashable #148

Merged
merged 6 commits into from
Oct 9, 2024
Merged

Conversation

Miauwkeru
Copy link
Contributor

No description provided.

Copy link

codecov bot commented Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.92%. Comparing base (4e1a285) to head (4a21763).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #148      +/-   ##
==========================================
+ Coverage   83.89%   83.92%   +0.03%     
==========================================
  Files          34       34              
  Lines        3458     3466       +8     
==========================================
+ Hits         2901     2909       +8     
  Misses        557      557              
Flag Coverage Δ
unittests 83.92% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Schamper Schamper changed the title Add __hash__ to ipnetwork and ipaddress Make ipnetwork and ipaddress hashable Oct 8, 2024
flow/record/fieldtypes/net/ip.py Outdated Show resolved Hide resolved
flow/record/fieldtypes/net/ip.py Show resolved Hide resolved
flow/record/fieldtypes/net/ip.py Outdated Show resolved Hide resolved
flow/record/fieldtypes/net/ip.py Outdated Show resolved Hide resolved
flow/record/fieldtypes/net/ip.py Outdated Show resolved Hide resolved
flow/record/fieldtypes/net/ip.py Outdated Show resolved Hide resolved
tests/test_fieldtype_ip.py Show resolved Hide resolved
Copy link
Member

@yunzheng yunzheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you change to new style typing

flow/record/fieldtypes/net/ip.py Outdated Show resolved Hide resolved
@Miauwkeru
Copy link
Contributor Author

can you change to new style typing

it will break the python3.8 compatibility if I change it to new style typing, which is why i haven't done so.

@yunzheng
Copy link
Member

yunzheng commented Oct 9, 2024

can you change to new style typing

it will break the python3.8 compatibility if I change it to new style typing, which is why i haven't done so.

should work fine due to from __future__ import annotations

@Miauwkeru
Copy link
Contributor Author

can you change to new style typing

it will break the python3.8 compatibility if I change it to new style typing, which is why i haven't done so.

should work fine due to from __future__ import annotations

Ah, I see I checked it wrong. I tried x = str | int that doesn't work, but x: str | int = None does work. So its during assigning that it went wrong. My bad, changed the definitions.

Copy link
Member

@yunzheng yunzheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some small suggestions and then it should be good to go!

flow/record/fieldtypes/net/ip.py Outdated Show resolved Hide resolved
flow/record/fieldtypes/net/ip.py Outdated Show resolved Hide resolved
@yunzheng yunzheng dismissed Schamper’s stale review October 9, 2024 11:49

already resolved

@yunzheng yunzheng merged commit c4e5641 into main Oct 9, 2024
20 checks passed
@yunzheng yunzheng deleted the make-net-types-hashable branch October 9, 2024 11:50
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