Hi @777GE90 ,

Could you share some motivation references that could help explain why this next_network method will be helpful? Is this common enough to warrant a method in stdlib IP-Address module? If there is prior-art in other external libraries or libraries of other languages, that will be helpful to know and review too.

Thanks!

Here are the motivations for getting this added:
FEELS INCOMPLETE: This library includes the subnet_of and supernet_of methods, they will effectively allow you to break down a network into smaller subnets (i.e. go down) or find the networks parent network (i.e. go up). But what they are missing is finding the next closest network (i.e. go sideways) - (which reminds me, perhaps I should implement a prev_network() as well?). It's always been a case where this library has felt incomplete as it lets you go in the up and down direction but omits the sideways direction.
IT'S COMPLICATED: Finding the next closest network is not a trivial task to do in your head and it's difficult to understand, from my experience the most efficient way of doing it is on the binary level. When I first had a use case to do this, it took me a good amount of time trying to understand this algorithm and implement it, I'm certain there will be many people out there struggling with this and reinventing the wheel. By adding this into the ipaddress module, we help simplify this task and improve peoples network knowledge. A lot of users of the ipaddress module are likely to be Network Engineers who have little programming experience, they would probably not be experienced enough to implement something like this or get involved in feeding it back to the Python project.
IT'S NEEDED: Personally, I almost exclusively work with IPv4 networks and one very common use case we get is slicing up networks to use them as efficiently as possible. For example, if I am given a network of 10.250.1.0/16 and I need to reserve a /21 for some servers within that range, I would initially just reserve at the start of the network, i.e. 10.250.1.0/21 - very simple. But now, say someone comes along and says they want a /19 from the same network, then it's not so simple anymore. The 10.250.1.0/21 network runs from 10.250.1.0-10.250.7.254 - you can't just take the broadcast address and add 1 to it (i.e. 10.250.8.0/19 would be invalid), the next closest /19 network is actually infact 10.250.32.0/19. There is no way to figure this out currently in the ipaddress module, even though it is a very common practice that is done by network engineers manually. A helpful way to illustrate this is by playing around on this VPC Subnet Builder here: https://tidalmigrations.com/subnet-builder/

I hope this helps - please let me know if you need anything else. I would not have spent any time on this if I didn't think it was a valuable and fitting element of this module. To me it seemed like a very important omission, but that's just my two cents.

Thank you for the reply, @777GE90 . I think, I have enough information so to study this a bit further, and review and possibly merge it.

Thanks for the detailed explanation, but I think a follow up question is, would this be useful programatically or just as an interactive tool? Because the use case you describe seems to be just interactive ilke the link you provide. In that case it seems like you would end up with 1-2 interactive tools that everyone uses and that use the logic in this PR, and in that scenario it would be more appropriate if such tool contained this code directly.

Not sure if I understood your question correctly, but if your question is whether this function is limited to 1-2 interactive tools then the answer is no, the interactive tool I linked is merely a visual aid to help illustrate what this method does.

There are many use cases for this function (another example is using it for dynamic network allocation) and I would say it's no less valid than other functions in this library, such as subnet_of and supernet_of methods I previously mentioned. You could argue this whole library doesn't need to be part of the standard CPython, but that argument is beyond the scope of my PR. This library exists and we should maintain it whilst it does.

We have actually had a discussion regarding this PR through e-mails a few months back, which unfortuntely I guess you didn't have the benefit of viewing. My plans are to raise another PR with a prev_network method as well and take into account any comments from this one, however I lost my development VM and haven't had the time since. Hopefully this is something I can do in the next couple of weeks and then give @orsenthil a nudge to take a look for me if he can :).

I think this should be 3.11 now?

:( - is it too late for a potential 3.10 release now?

What I can do is add the prev_network method as well and raise another PR for this on the weekend, hopefully we can get this pushed through.

And this should be moved to 3.11

I think this is more readable if written as a normal if: .. else: .. block.

I think it's more readable and more available to linter tools to have method calls outside of f-strings, so something like ip = self._string_from_ip_int(next_ip); ... ; f"{ip}/{next_prefix}"

This formatting (this applies to the entire method) seems to be very inconsistent with the rest of the module (maybe auto-formatted?) I'm not sure what the policy of formatting is for Python code, in addition to pep8, I think it should be somewhat consistent with the module, but I could be wrong, so just noting it here..

The formatting was probably done with a Black formatter which uses PEP8 standards, if I remember correctly. So it is valid but I suppose potentially not consistent with other code in this module, however I did find that there are already inconsitencies in this module so didn't think it was a big deal, I will however look to make this more consistent in my next PR.

PEP 8 is a document for human interpretation first. One of the rules in there is to follow existing style in a module. There are tools like pep8 or black that pretend to enforce rigid rules mechanically, but CPython doesn’t use them and follow the spirit of PEP 8 including the “existing style” clause 🙂

For the caller, would it be useful to sometimes separately handle overflow error vs. value errors? I think (but could be wrong) that value errors are generally raised when inputs have an invalid value, so keeping overflow error here might be more technically accurate.

Thanks for the detailed explanation, but I think a follow up question is, would this be useful programatically or just as an interactive tool? Because the use case you describe seems to be just interactive ilke the link you provide. In that case it seems like you would end up with 1-2 interactive tools that everyone uses and that use the logic in this PR, and in that scenario it would be more appropriate if such tool contained this code directly.