isi-vista / immutablecollections

A library for immutable collections, in the spirit of Guava's Immutable Collections.
MIT License
3 stars 1 forks source link

Fix inability to initialize immutableset from KeysView on recent Pythons #36

Closed gabbard closed 5 years ago

gabbard commented 5 years ago

Closes #35

codecov[bot] commented 5 years ago

Codecov Report

Merging #36 into master will increase coverage by 0.33%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #36      +/-   ##
==========================================
+ Coverage   90.17%   90.51%   +0.33%     
==========================================
  Files           7        7              
  Lines         621      622       +1     
==========================================
+ Hits          560      563       +3     
+ Misses         61       59       -2
Impacted Files Coverage Δ
immutablecollections/_immutableset.py 90.99% <100%> (+0.99%) :arrow_up:

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 6ffb09a...5edc5d1. Read the comment docs.

gabbard commented 5 years ago

@berquist : I'm going to be out the next two days; can you add the tests @ConstantineLignos requests? I think you'll need to find a way to restrict tests to certain Python versions (or do the check within the test)

berquist commented 5 years ago

The reason why the conditional is split onto separate lines rather than combined is that despite short circuit evaluation, there is a small performance hit from using a combined conditional:

In [1]: %%timeit
   ...: if False and isinstance(5, object):
   ...:     pass
   ...:
9.74 ns ± 0.218 ns per loop (mean ± std. dev. of 7 runs, 100000000 loops each)

In [2]: %%timeit
   ...: if False:
   ...:     if isinstance(5, object):
   ...:         pass
   ...:
6.46 ns ± 0.0819 ns per loop (mean ± std. dev. of 7 runs, 100000000 loops each)

In [5]: %timeit 5 is False and True
18.3 ns ± 0.4 ns per loop (mean ± std. dev. of 7 runs, 100000000 loops each)

In [3]: %timeit 5 is False
15.9 ns ± 0.145 ns per loop (mean ± std. dev. of 7 runs, 100000000 loops each)

immutablecollections benchmarks:

gabbard commented 5 years ago

Thanks @berquist !