Forbid assigning to a slice #1629

[sudo-k-runner]

Forbid assigning to a slice #1629

Checklist

• I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
• I have created at least one test case for the changes I have made
• I have updated the documentation for the changes I have made
• I have added my changes to the CHANGELOG.md

Related issues

Closes #1629

🙏 Please, if you or your company is finding wemake-python-styleguide valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/wemake-python-styleguide. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.

[codecov]

Codecov Report

Merging #1661 (0ee81b8) into master (dce62cb) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #1661   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          107       107           
  Lines         5992      6005   +13     
  Branches      1343      1346    +3     
=========================================
+ Hits          5992      6005   +13     

Impacted Files	Coverage Δ
wemake_python_styleguide/violations/consistency.py	100.00% <100.00%> (ø)
...emake_python_styleguide/visitors/ast/subscripts.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dce62cb...1b9ed1a. Read the comment docs.

[sobolevn]

Thanks a lot! 👍

[kxepal]

Sorry to be late for the party, but why? Slice assignment is the only way for inplace array multiple replacement when you need that. Any other alternatives on the table? Shouldn't any forbid rule follow with howto one?

And no, explicit index assignment isn't an alternative since you replace one shot operation which could be optimized by underlying structure (like numpy.array) via O(N) one which wouldn't much be optimal in most of cases.

[sobolevn]

It can be switched off for numpy array. In regular python this does not make any sence.

[kxepal]

The sense is quite the same: provide the way for multiple sequential items replacement in a single shot.

Dummy example how it works:

In [1]: l = list(range(100500))                                                                                                                                                                                                               

In [2]: def a(l, r=list(range(100,500))): 
   ...:     l[100:500] = r 
   ...:                                                                                                                                                                                                                                       

In [3]: def b(l, r=list(range(100,500))): 
   ...:     for i in r: 
   ...:         l[i] = i 
   ...:                                                                                                                                                                                                                                       

In [4]: %timeit a(l)                                                                                                                                                                                                                          
1.09 µs ± 27.4 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

In [5]: %timeit b(l)                                                                                                                                                                                                                          
14.4 µs ± 268 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

So at least this rule forces to do not an optimal operations. Any other reasons to not do slice assignments?

[kxepal]

The quite common and useful example which violates this rule is inplace list replacement via [:] - this helps to keep the same object reference while it content could be completely erased or replaced with the new one. Shared list thing.

[sobolevn]

@kxepal thanks a lot! This is a valuable addition.

The main idea of this rule is that list is mutable. And assigning to a slice changes the size of the list implicitly.

>>> l = [0, 1, 2, 3, 4]
>>> l[0:2]
[0, 1]
>>> l[0:2] = ['a', 'b', 'c']
>>> l
['a', 'b', 'c', 2, 3, 4]

And this is really confusing. How can I tell the final size of the list after l[0:2] = some()?
We can't even do this with mypy.

I think that this code is ugly. And I would be really unpleased to something like it in our production code.
On the other hand:

# We need this operation to be fast, because this is a hot path,
# so we use a hack to insert a part of another list in one bunch.
new_list_part = some()
assert len(new_list_part) == 500 - 100
my_list[100:500] = some_call()  # noqa: WPS-whatever

This code looks reasonable to me.

[sobolevn]

@sudo-k-runner can you please

1. rebase your branch on current master?
2. add bits from our discussion with @kxepal to the docs?

Thanks!

[sudo-k-runner]

@sobolevn Sure, apologies for the delay, got side tracked. Will updated the PR.

[sobolevn]

Great work, @sudo-k-runner
Thanks!