IPAddressField improvements

[andreagrandi]

This PR is an extension of this original one #2618

What I did was reviewing the code and adding a couple of test cases to the IPAddressField.

This code was produced as part of the Django Sprint hosted by Potato in London: http://djangosprint.london/

[tomchristie]

Looking good. I think we arrived at a slight tweak to this, that involved us getting rid of the unpack_ipv4 flag and just always having that as True for 'both'. Something like this?...

if self.protocol in ('both', 'ipv6'):
    unpack_ipv4 = (self.protocol == 'both')
    return clean_ipv6_address(data)
return data

[andreagrandi]

Hi Tom,

I'm away from my computer for few days so I will be able to reply this more properly at the end of the week. Basically I noticed that ipv4_unpack was needed anyway by the method that returns validator, that's why I left it. I need to check if tests still pass always defaulting it to True when is both. It doesn't have to be always True, nor always false.

[andreagrandi]

I'm now back from holidays. I've given a look to the methods. The problem we may have forcing the unpack_ipv4 in case of 'both' is the following:

• the user choose to use 'both' because he/she needs to validate both ipv4 and ipv6.
• the use passes '192.168.0.1' as value, and the value is automatically 'unpacked'.
• the output of 192.168.0.1 is: 2002:c0a8:0001::c0a8:0001

are you sure this is what we want?

[andreagrandi]

Actually I got this wrong again (oh gosh... I really suck with this!). unpack_ipv4 does exactly the opposite: given an ip that is an ipv6 representation of an ipv4, unpack_ipv4 transforms back the ipv6 to ipv4. This is proven by these tests https://github.com/django/django/blob/0ed7d155635da9f79d4dd67e4889087d3673c6da/tests/utils_tests/test_ipv6.py#L53

[andreagrandi]

@tomchristie I apologize for the long delay in fixing this. It should now be ok to be merged.

[jpadilla]

LGTM.

@tomchristie @xordoquy for when should we milestone this, 3.1.2 or 3.2.0?

[jpadilla]

Milestoned this as good to go for 3.2.0

[tomchristie]

No problem from me with this going into a minor release instead, if that's better with @xordoquy. Either way okay.

[tomchristie]

Pushing this forward to either 3.1.3 (or 3.1.4 if needed)
Don't see any need to wait for 3.2.0 for it.
Needs updating to master, but otherwise looks good to go.

[xordoquy]

I'll update and merge this tonight.