Improving conciseness of ipaddress factory functions.
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 77.2k
- Forks
- 36k
- PR merge metrics
- PR metrics pending
Description
Suggestion to refactor the 3 factory functions ip_address, ip_network, ip_interface.
In addition making a similar modification in the _prefix_from_ip_string function.
Currently the functions contain code which can be improved in terms of conciseness and extendability.
try/catchcontaining apassstatement can be substituted withcontextlib.suppressmaking the code more concise- we can use a
forloop which will iterate over theclassobjects to further increase conciseness
Current factory function implementations (docstrings not included for brevity)
def ip_address(address):
try:
return IPv4Address(address)
except (AddressValueError, NetmaskValueError):
pass
try:
return IPv6Address(address)
except (AddressValueError, NetmaskValueError):
pass
raise ValueError(f'{address!r} does not appear to be an IPv4 or IPv6 address')
def ip_network(address, strict=True):
try:
return IPv4Network(address, strict)
except (AddressValueError, NetmaskValueError):
pass
try:
return IPv6Network(address, strict)
except (AddressValueError, NetmaskValueError):
pass
raise ValueError(f'{address!r} does not appear to be an IPv4 or IPv6 network')
def ip_interface(address):
try:
return IPv4Interface(address)
except (AddressValueError, NetmaskValueError):
pass
try:
return IPv6Interface(address)
except (AddressValueError, NetmaskValueError):
pass
raise ValueError(f'{address!r} does not appear to be an IPv4 or IPv6 interface')
Proposed function implementations
def ip_address(address):
for ip_class in [IPv4Address, IPv6Address]:
with contextlib.suppress(AddressValueError, NetmaskValueError):
return ip_class(address)
raise ValueError(f'{address!r} does not appear to be an IPv4 or IPv6 address')
def ip_network(address, strict=True):
for network_class in [IPv4Network, IPv6Network]:
with contextlib.suppress(AddressValueError, NetmaskValueError):
return network_class(address, strict)
raise ValueError(f'{address!r} does not appear to be an IPv4 or IPv6 network')
def ip_interface(address):
for ip_interface_class in [IPv4Interface, IPv6Interface]:
with contextlib.suppress(AddressValueError, NetmaskValueError):
return ip_interface_class(address)
raise ValueError(f'{address!r} does not appear to be an IPv4 or IPv6 interface')
current _prefix_from_ip_string function (docstrings and comments not included for brevity)
@classmethod
def _prefix_from_ip_string(cls, ip_str):
try:
ip_int = cls._ip_int_from_string(ip_str)
except AddressValueError:
cls._report_invalid_netmask(ip_str)
try:
return cls._prefix_from_ip_int(ip_int)
except ValueError:
pass
ip_int ^= cls._ALL_ONES
try:
return cls._prefix_from_ip_int(ip_int)
except ValueError:
cls._report_invalid_netmask(ip_str)
Suggested improvement
@classmethod
def _prefix_from_ip_string(cls, ip_str):
try:
ip_int = cls._ip_int_from_string(ip_str)
except AddressValueError:
cls._report_invalid_netmask(ip_str)
with contextlib.suppress(ValueError):
return cls._prefix_from_ip_int(ip_int)
ip_int ^= cls._ALL_ONES
try:
return cls._prefix_from_ip_int(ip_int)
except ValueError:
cls._report_invalid_netmask(ip_str)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Locate the ip_address, ip_network, ip_interface, and _prefix_from_ip_string entry points in the ipaddress implementation, then read their surrounding exception handling. Compare the proposed loop and suppression changes against the current behavior, including the existing error paths, and verify that the relevant ipaddress tests still pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- networking
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100