-
Notifications
You must be signed in to change notification settings - Fork 756
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
E251: triggers on line continuations #301
Comments
This is interesting. This changed in version 1.5.2. Prior to that, it was not an error. |
Looks like this was changed in 0a10f3c |
It was added as a result of #252 with which I'm not sure I disagree. An argument could be made that the newline character is a whitespace character covered under the rule. One could also argue (if you disagree about a newline character being whitespace) that the indentation before the value would be whitespace, but I'm not really interested in arguing. @ncoghlan what do you think about this (given #256 does not include it)? |
Personally, pep8 is flexible enough that I've always preferred simply changing the function/method call so that the parameters are all only indented once, e.g., parser.add_argument('--long-option',
default="/rather/long/filesystem/path/here/blah/blah/blah")
# Changes to
parser.add_argument(
'--long-option',
default="/rather/long/filesystem/path/here/blah/blah/blah"
) |
What would you do in this case: http://git.openstack.org/cgit/openstack/nova/tree/nova/tests/network/test_manager.py#n239 |
Code in question, so that when that file changes the example isn't lost. vif_dict = dict(bridge=network['bridge'],
cidr=net_v4['cidr'],
cidr_v6=net_v6['cidr'],
id=vif['id'],
multi_host=network.get_meta('multi_host', False),
injected=network.get_meta('injected', False),
bridge_interface=
network.get_meta('bridge_interface'),
vlan=network.get_meta('vlan'),
broadcast=str(net_v4.as_netaddr().broadcast),
dhcp_server=network.get_meta('dhcp_server',
net_v4['gateway']['address']),
dns=[ip['address'] for ip in net_v4['dns']],
gateway=net_v4['gateway']['address'],
gateway_v6=net_v6['gateway']['address'],
label=network['label'],
mac=vif['address'],
rxtx_cap=vif.get_meta('rxtx_cap'),
vif_type=vif['type'],
vif_devname=vif.get('devname'),
vif_uuid=vif['id'],
ovs_interfaceid=vif.get('ovs_interfaceid'),
qbh_params=vif.get('qbh_params'),
qbg_params=vif.get('qbg_params'),
should_create_vlan=
network.get_meta('should_create_vlan', False),
should_create_bridge=
network.get_meta('should_create_bridge',
False),
ip=net_v4['ips'][i]['address'],
ip_v6=net_v6['ips'][i]['address'],
netmask=str(net_v4.as_netaddr().netmask),
netmask_v6=net_v6.as_netaddr()._prefixlen,
physical_network=
network.get_meta('physical_network', None))
self.assertThat(vif_dict, matchers.DictMatches(check)) |
@sigmavirus24 missing (edited by @florentx) thanks, I've added the missing quote |
@jogo I hesitate to revert the patch for #252 . If you are looking for alternative syntaxes, I could propose:
vif_dict = dict(
bridge=network['bridge'],
cidr=net_v4['cidr'],
cidr_v6=net_v6['cidr'],
id=vif['id'],
multi_host=network.get_meta('multi_host', False),
injected=network.get_meta('injected', False),
bridge_interface=network.get_meta('bridge_interface'),
#
}
multi_host = network.get_meta('multi_host', False)
bridge_interface = network.get_meta('bridge_interface')
vif_dict = dict(
bridge=network['bridge'],
cidr=net_v4['cidr'],
cidr_v6=net_v6['cidr'],
id=vif['id'],
multi_host=multi_host,
injected=network.get_meta('injected', False),
bridge_interface=bridge_interface,
#
} There's always a solution... |
@florentx switching to hanging indentation instead of visual indentation would work, but I am still confused about the fix for #252, it doesn't sound like your fix fully addresses the request (missing support for extra long lines -- something I am sure folks won't universally accept). While switching to hanging indent would work, I am concerned that this rule goes beyond what pep8 requires. Since this is "a tool to check your Python code against some of the style conventions in PEP 8." going beyond the pep8 scope in the default settings. |
@sigmavirus24 agreed |
Here's a compromise on this issue: What if the behaviour (newlines around the assignment operator) becomes a new error code, e.g., E252. If that were to happen, either it could be off by default or OpenStack could ignore it more easily without ignoring the very valuable E251. I am not entirely convinced that the result of #252 is a bug fix. It seems (obviously, in my opinion) more like an added feature and as such probably shouldn't have been released in 1.5.x but instead in 1.6.0. Fixing that would be fine in 1.5.x and fixing it in this manner would be fine as well. I'm also aware and appreciative of the hesitancy with which @florentx adds new error codes. |
@sigmavirus24 that sounds like a great plan to me. I agree splitting the result of #252 out into its own check would be great, especially if it was off by default. |
Project: openstack/nova 98636a486d5a7bfd35004b02a893fd45ed065323 Cleanup and gate on pep8 rules that are stricter in hacking 0.9 Fix up pep8 failures for rules we previously gated on, but that are now stricter in there enforcement. Ignore E251 due to PyCQA/pycodestyle#301 Change-Id: I62b33e97c44c4a5be436b381cfbdaeb31cd2638b
Fix up pep8 failures for rules we previously gated on, but that are now stricter in there enforcement. Ignore E251 due to PyCQA/pycodestyle#301 Change-Id: I62b33e97c44c4a5be436b381cfbdaeb31cd2638b
Any updates on this? This is particularly annoying when using |
@sk- none since 2014. |
I encounter the same issue when using |
any updates? same problem with yapf & flake8 |
commenting asking for updates is a waste of everyone's time, please don't do that @MerdyNumber1 if there were updates you would see either a comment here or a linked issue. in the future please use the voting feature instead of bumping threads needlessly |
Unless I am missing something this should be allowed by E251. Since E251 is
" E251 unexpected spaces around keyword / parameter equals" and there are no extra spaces here
The text was updated successfully, but these errors were encountered: