-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
bpo-38335 simplify the overlap function for IpNetwork #16519
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: main
Are you sure you want to change the base?
Conversation
no news entry needed as there is no behavior change |
…global for network
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @s-sanjay, you should not move the method around in the file while changing them as it make reviewing the changes harder. Can you please move them back where they were?
@@ -1511,7 +1512,6 @@ def __init__(self, address, strict=True): | |||
self.hosts = self.__iter__ | |||
|
|||
@property | |||
@functools.lru_cache() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you removed this decorator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because self.network_address.is_global which in turn calls is_private already has a cache. There are pros and cons for example if someone removes cache there then suddenly this would loose caching behavior and also this having its own cache frees up the other cache. Should I add it back ?
@remilapeyre yes, agreed I avoid moving methods around but in this case the is_private method was deleted from child classes and moved to the base class and the overlap method was moved closer to the subnet_of and supernet_of functions because overlap and them are related terms and the overlap method simply delegates the call to these two methods. I thought this small refactor will help greatly with future readability of the code. what do you think ? should I put it in the old place ? |
The following commit authors need to sign the Contributor License Agreement: |
https://bugs.python.org/issue38335