isi-vista / immutablecollections

A library for immutable collections, in the spirit of Guava's Immutable Collections.

MIT License

3 stars 1 forks source link

Provide ImmutableDict & ImmutableMultiDict factory methods which ban duplicate additions (Closes issue #53) #55

Closed qpwo closed 5 years ago

qpwo commented 5 years ago

Initial draft. I forgot some things in PR 54 that @ConstantineLignos pointed out; should I fix those in this PR?

berquist commented 5 years ago

https://github.com/isi-vista/immutablecollections/pull/54#discussion_r292199026 ? Yes

codecov[bot] commented 5 years ago

Codecov Report

Merging #55 into master will increase coverage by 0.1%. The diff coverage is 94.73%.

@@            Coverage Diff            @@
##           master      #55     +/-   ##
=========================================
+ Coverage   90.66%   90.76%   +0.1%     
=========================================
  Files           6        6             
  Lines         621      639     +18     
=========================================
+ Hits          563      580     +17     
- Misses         58       59      +1

Impacted Files	Coverage Δ
immutablecollections/_immutableset.py	`90.47% <100%> (+0.12%)`	:arrow_up:
immutablecollections/_immutabledict.py	`87.8% <93.33%> (+0.76%)`	: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 e1daaa9...14080a9. Read the comment docs.

qpwo commented 5 years ago

Do we want immutablesetmultidict_from_unique_keys and immutablelistmultidict_from_unique_keys and associated kwargs?

berquist commented 5 years ago

Probably not, because the point of the multidicts is to associate a single key with multiple values. A user would fully expect multiple keys to appear.

qpwo commented 5 years ago

Oh I think this might be ready to merge then

qpwo commented 5 years ago

Did you already make tests for ImmutableSet?

Yes that was in PR 54

gabbard commented 5 years ago

These are the coding standards: https://github.com/isi-nlp/isi-flexnlp/blob/master/docs/coding_standards.rst

I don't see the "boolean arguments should always be keywords" rule there, but I remember discussing it. @berquist , can you add it?

ConstantineLignos commented 5 years ago

@rgabbard Discussion was here, btw: https://github.com/isi-nlp/isi-flexnlp/issues/266#issuecomment-360886337 . (You can see me being quite confused about how this works in Python in that discussion :).)