Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (625a780) 85.72% compared to head (53b8974) 86.01%.
Report is 11 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master     #376      +/-   ##
==========================================
+ Coverage   85.72%   86.01%   +0.28%     
==========================================
  Files          47       47              
  Lines        4379     4390      +11     
  Branches      737      740       +3     
==========================================
+ Hits         3754     3776      +22     
+ Misses        449      436      -13     
- Partials      176      178       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Thank you for this. FWIW as I mentioned in #123 (comment) I'm currently undecided on this feature.

Flags in python never quite felt right to me either. I tried to be as consistant with the existing codebase as possible, such as the IPAddress constructor.

Otherwise I could reimpliment this with a tuple flags, seperate parameter, a class method, or some other way if you have any ideas. A few interface examples:

nwk = IPNetwork('192.168.0.1/24', flags=NOHOST | HOSTMASK)
nwk = IPNetwork('192.168.0.1/24', flags=(NOHOST, HOSTMASK))
nwk = IPNetwork('192.168.0.1/24', nohost=True, hostmask=True)

If you feel this is a bad fit I understand you closing the MR.

To clarify, I'm hesitant regarding the feature itself (supporting ACL masks) – the way to trigger the feature is secondary, although I share your thoughts about integer flags here.

Unfortunately cisco did decide to use acl masks, so they are out there and likely never going away. I am currently splitting off the ACL masks then applying a hack before passing it to the IPNetwork constructor. Its works but is kind of a pain.

Could you provide more detail as to why you are hesitant regarding the feature itself?

I'm just unsure if the benefit is worth the increased interface complexity cost. Maybe this is used/needed more commonly then I think though.

Interface-wise I think something like

nwk = IPNetwork('192.168.0.1/24', nohost=True, hostmask=True)

(nohost and hostmask keyword-only, names to be adjusted as needed)

would be better than the current flag system.

If that is the case then I can (with your approval)

• Make a new MR to add keyword flags to all classes
  • Update the documentation and mark the flags field a deprecated
• Update this MR to do the same (removing the HOSTMASK constant)

I think let's hold on on changing the existing flags – I need to think about this some more.

This PR though – please change the flag to a keyword-only parameter.

One note – we definitely need a doctest in IPNetwork to show how the parameter affects parsing (and maybe mention Cisco and ACLs too).

Thank you.

I am going to have to strongly disagree here. I think it would be a really bad idea to split the API and have some flags and some keyword-only parameters. There should be one clear consistant way to call the methods.

Edit: There is already a doctest using flags. I could add a note about Cisco and ACLs.
https://github.com/netaddr/netaddr/pull/376/files#diff-a89567e4b1edf284bb7d536df42bfc0d6ac51b2ac9b86cd3bca746169aab4b82R999-R1002