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

Allow IPNetwork to take a tuple #60735

Closed
pitrou opened this issue Nov 22, 2012 · 15 comments
Closed

Allow IPNetwork to take a tuple #60735

pitrou opened this issue Nov 22, 2012 · 15 comments
Labels
easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@pitrou
Copy link
Member

pitrou commented Nov 22, 2012

BPO 16531
Nosy @ncoghlan, @pitrou, @ezio-melotti, @asvetlov, @serhiy-storchaka
Files
  • issue16531.patch
  • issue16531-2.patch
  • issue16531-3.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2014-05-12.18:37:22.497>
    created_at = <Date 2012-11-22.21:59:26.238>
    labels = ['easy', 'type-feature', 'library']
    title = 'Allow IPNetwork to take a tuple'
    updated_at = <Date 2014-05-12.18:37:22.496>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2014-05-12.18:37:22.496>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-05-12.18:37:22.497>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2012-11-22.21:59:26.238>
    creator = 'pitrou'
    dependencies = []
    files = ['28233', '28402', '35229']
    hgrepos = []
    issue_num = 16531
    keywords = ['patch', 'easy']
    message_count = 15.0
    messages = ['176129', '176137', '176151', '176159', '177025', '177067', '177114', '177115', '177934', '177964', '189205', '218341', '218345', '218346', '218347']
    nosy_count = 9.0
    nosy_names = ['ncoghlan', 'pitrou', 'jongfoster', 'ezio.melotti', 'pmoody', 'asvetlov', 'python-dev', 'serhiy.storchaka', 'Kiyoshi.Aman']
    pr_nums = []
    priority = 'low'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue16531'
    versions = ['Python 3.5']

    @pitrou
    Copy link
    Member Author

    pitrou commented Nov 22, 2012

    I am in a situation where I'm building an IPNetwork from separate address and mask information. So roughly I'd like to write either:

     addr = IPAddress('192.168.0.0')
     network = IPNetwork((addr, '255.255.0.0'))

    or

     addr = '192.168.0.0'
     network = IPNetwork((addr, '255.255.0.0'))

    Of course it seems like this would be equivalent to:

     network = IPNetwork('%s/%s' % (addr, '255.255.0.0.'))

    (but more user-friendly :-))

    @pitrou pitrou added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Nov 22, 2012
    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Nov 22, 2012

    Sounds reasonable, especially as it also allows networks and interfaces
    with prefixes other than "/32" or "/128" to be easily constructed based on
    integer address values.

    Should we also allow integers as the second argument, with the same prefix
    length meaning as the corresponding string?

    @pitrou
    Copy link
    Member Author

    pitrou commented Nov 23, 2012

    Sounds reasonable, especially as it also allows networks and interfaces
    with prefixes other than "/32" or "/128" to be easily constructed based on
    integer address values.

    Should we also allow integers as the second argument, with the same prefix
    length meaning as the corresponding string?

    IMO, yes.

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Nov 23, 2012

    How about ipaddress.IPv4Network((3232235520, 16)), ipaddress.IPv4Network((3232235520, 65535)) and ipaddress.IPv4Network((3232235520, 4294901760))?

    >>> ipaddress.IPv4Address(3232235520)
    IPv4Address('192.168.0.0')
    >>> ipaddress.IPv4Address(65535)
    IPv4Address('0.0.255.255')
    >>> ipaddress.IPv4Address(4294901760)
    IPv4Address('255.255.0.0')
    >>> ipaddress.IPv4Network('192.168.0.0/0.0.255.255')
    IPv4Network('192.168.0.0/16')
    >>> ipaddress.IPv4Network('192.168.0.0/255.255.0.0')
    IPv4Network('192.168.0.0/16')

    @pmoody
    Copy link
    Mannequin

    pmoody mannequin commented Dec 6, 2012

    on it.

    I'm not a huge fan of integer args for the first argument because of possible confusion between v4/v6.

    @pmoody
    Copy link
    Mannequin

    pmoody mannequin commented Dec 7, 2012

    This patch is for (address, prefixlen). I now see that the original request was (address, netmask). I'll fix this up. In the meantime, let me know if this is what you had in mind.

    Cheers,
    peter

    @pitrou
    Copy link
    Member Author

    pitrou commented Dec 7, 2012

    This patch is for (address, prefixlen). I now see that the original
    request was (address, netmask). I'll fix this up. In the meantime, let
    me know if this is what you had in mind.

    This is what I had in mind indeed (except that I was mostly interested in the netmask case :-)).

    @pitrou
    Copy link
    Member Author

    pitrou commented Dec 7, 2012

    Oh, I was also interested in IPNetwork((ip_address_object, netmask)).

    (that is, given an existing IPAddress object, build a IPNetwork by passing that object + a netmask)

    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Dec 22, 2012

    IIRC, construction from existing instances in general is an issue in the current version of the module. One particularly murky question if it were allowed is what should happen if you pass an IPAdapter instance (which already has associated network info) rather than an ordinary IPAddress.

    Forcing people to go through an integer or a string in that case, or call one of the explicit conversion methods, forces them to be explicit about the fact that they're deliberate discarding any other information associated with the original object. (Note that I like the 2-tuple idea - I'm just pointing out that allowing an IPAddress object as the first element of that 2-tuple isn't quite as straightforward as it may first appear)

    @pmoody
    Copy link
    Mannequin

    pmoody mannequin commented Dec 23, 2012

    Sorry. I thought I'd uploaded this earlier. I'm working on a version that pushes the parsing into _split_optional_netmask

    @KiyoshiAman
    Copy link
    Mannequin

    KiyoshiAman mannequin commented May 14, 2013

    I would instead suggest a cidr or netmask kwarg, rather than accepting a tuple as first argument.

    @pitrou
    Copy link
    Member Author

    pitrou commented May 12, 2014

    Updated patch with more tests, documentation updates, and an optimized version of the supernet() that's now 5x faster than the original.

    I would like to commit this if there's no further comment.

    @pmoody
    Copy link
    Mannequin

    pmoody mannequin commented May 12, 2014

    fine by me. this has been on my todo list forever by $payingjob and $family have prevented me from devoting any time.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 12, 2014

    New changeset 4e33c343a264 by Antoine Pitrou in branch 'default':
    Issue bpo-16531: ipaddress.IPv4Network and ipaddress.IPv6Network now accept an (address, netmask) tuple argument, so as to easily construct network objects from existing addresses.
    http://hg.python.org/cpython/rev/4e33c343a264

    @pitrou
    Copy link
    Member Author

    pitrou commented May 12, 2014

    Thanks for the approval, Peter!

    @pitrou pitrou closed this as completed May 12, 2014
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants