Skip to content

bpo-38335 simplify the overlap function for IpNetwork#16519

Open
s-sanjay wants to merge 2 commits intopython:mainfrom
s-sanjay:master
Open

bpo-38335 simplify the overlap function for IpNetwork#16519
s-sanjay wants to merge 2 commits intopython:mainfrom
s-sanjay:master

Conversation

@s-sanjay
Copy link
Copy Markdown
Contributor

@s-sanjay s-sanjay commented Oct 1, 2019

@s-sanjay
Copy link
Copy Markdown
Contributor Author

s-sanjay commented Oct 1, 2019

no news entry needed as there is no behavior change

Copy link
Copy Markdown

@remilapeyre remilapeyre left a 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?

self.hosts = self.__iter__

@property
@functools.lru_cache()
Copy link
Copy Markdown

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?

Copy link
Copy Markdown
Contributor Author

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 ?

@s-sanjay
Copy link
Copy Markdown
Contributor Author

@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 ?

@python-cla-bot
Copy link
Copy Markdown

The following commit authors need to sign the Contributor License Agreement:

CLA signed

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review stale Stale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants