Add Data.Set module #239

Merged
merged 3 commits into from Mar 22, 2015

Projects

None yet

3 participants

@haya14busa
Member

python の 2.7 のset ライブラリと 3.4 の仕様(と少しソースコード)を参考に vitalに移植してみました

TODO

  • #239 (comment) (複数の引数に対応する場合)(可変長引数かメソッド以外の方法で提供するかそもそも対応しないかで選択肢があり、どうするか思案中。保留もあり?)
  • #239 (diff) (一意性をどうやって確保するか)
  • rebase (okそうであれば最後にやる)
@thinca thinca commented on an outdated diff Dec 7, 2014
doc/vital-data-set.txt
+INTRODUCTION *Vital.Data.Set-introduction*
+
+*Vital.Data.Set* is Collection Utilities Library.
+It provides set and frozenset data structure ported from python.
+ https://docs.python.org/3.4/library/stdtypes.html#set-types-set-frozenset
+
+
+==============================================================================
+INTERFACE *Vital.Data.Set-interface*
+------------------------------------------------------------------------------
+FUNCTIONS *Vital.Data.Set-functions*
+
+set([{list}]) *Vital.Data.Set.set()*
+ Returns a new 'Set' object.
+
+frozen([{list}]) *Vital.Data.Set.frozen()*
@thinca
thinca Dec 7, 2014 Member

実際の関数名は frozenset() になっているようです。

@thinca thinca commented on an outdated diff Dec 7, 2014
doc/vital-data-set.txt
+ Test whether every element in the set is in other.
+
+Set.lt({other}) *Vital.Data.Set-Set.lt()*
+ Test whether the set is a proper subset of other, that is, >
+ set.le(other) && set != other
+
+Set.issuperset({other}) *Vital.Data.Set-Set.issuperset()*
+Set.ge({other}) *Vital.Data.Set-Set.ge()*
+ Test whether every element in other is in the set.
+
+Set.gt({other}) *Vital.Data.Set-Set.gt()*
+ Test whether the set is a proper superset of other, that is, >
+ set.ge(other) && set != other
+
+Set.len() *Vital.Data.Set-Set.len()*
+ Return the length of Set
@thinca
thinca Dec 7, 2014 Member

文末に . がないです。

@thinca thinca and 1 other commented on an outdated diff Dec 7, 2014
doc/vital-data-set.txt
+set([{list}]) *Vital.Data.Set.set()*
+ Returns a new 'Set' object.
+
+frozen([{list}]) *Vital.Data.Set.frozen()*
+ Returns a new 'Frozen' object which have not mutable methods
+ (|Vital.Data.Set-Set-mutable|).
+
+------------------------------------------------------------------------------
+Set Object *Vital.Data.Set-Set*
+
+Set.to_list() *Vital.Data.Set-Set.to_list()*
+ Converts to |List|.
+
+Set.union({other}) *Vital.Data.Set-Set.union()*
+Set.or({other}) *Vital.Data.Set-Set.or()*
+ Return a new set with elements from the set and all others.
@thinca
thinca Dec 7, 2014 Member

引数は1つなので、all others は不自然かと思います。
あとここだけじゃないんですが、引数に言及する場合は文中でも {other} の表記を使った方がわかりやすいです。

@haya14busa
haya14busa Dec 7, 2014 Member

あー...! むしろdocumentに合わせて(引数の部分は変えるけど)複数otherを受け取れるようにコードを修正したほうが便利そうですね. 修正します

@thinca
thinca Dec 7, 2014 Member

可変長引数は慎重に使ってください。可変長でパラメータを受け取れるようにしてしまうと、今後オプショナルな引数の追加が困難になってしまいます。

@haya14busa
haya14busa Dec 7, 2014 Member

アー... どうしましょう。追加する可能性ありますかね

@thinca
thinca Dec 7, 2014 Member

例えば、Set オブジェクト側はこのままにして、モジュール側の関数として union({list-of-set}) とかを用意すると言う手もあります。

@haya14busa
haya14busa Dec 7, 2014 Member

んーなるほどそういう手もありますか...

@haya14busa
haya14busa Dec 7, 2014 Member

とりあえずはドキュメントを直しておきました

@thinca
Member
thinca commented Dec 7, 2014

Set でも重要なファクターの1つ、元にした Python のでいうところの x in s がない予感がします…!

@thinca
Member
thinca commented Dec 7, 2014

SetFrozenSet を混ぜて使った場合の戻り値について説明が欲しいです。

@thinca
Member
thinca commented Dec 7, 2014

すぐじゃなくていいと思いますが、一意性を定義するための関数 (恐らくハッシュ値を生成するものか、同一性の比較関数) が与えられると便利そうです。

@thinca thinca and 1 other commented on an outdated diff Dec 7, 2014
autoload/vital/__latest__/Data/Set.vim
+function! s:_base_set.len() abort
+ return len(self._data)
+endfunction
+
+function! s:_base_set.to_list() abort
+ return values(self._data)
+endfunction
+
+function! s:_base_set._update(xs) abort
+ if s:_is_set(a:xs)
+ call self.update(a:xs.to_list())
+ return
+ endif
+ for X in a:xs
+ call self._add(X)
+ unlet X | endfor
@thinca
thinca Dec 7, 2014 Member

なぜ | を使ってるんでしょう…?

@haya14busa
haya14busa Dec 7, 2014 Member

すいません単にその時の気分でした(わざわざ1行増えるのがその時は嫌だった). 修正します

@thinca thinca commented on an outdated diff Dec 7, 2014
autoload/vital/__latest__/Data/Set.vim
+" Remove and return an arbitrary set element.
+function! s:set.pop() abort
+ let key = keys(self._data)[0]
+ let e = self._data[key]
+ unlet self._data[key]
+ return e
+endfunction
+
+" Helper:
+
+function! s:_is_set(x) abort
+ return type(a:x) is type({}) && get(a:x, '_is_set', s:FALSE)
+endfunction
+
+function! s:_throw(message) abort
+ throw 'vital.Data.Sets: ' . a:message
@thinca
thinca Dec 7, 2014 Member

例外は vital: Data.Set: でお願いします。

@thinca thinca and 1 other commented on an outdated diff Dec 7, 2014
autoload/vital/__latest__/Data/Set.vim
+function! s:_base_set.gt(t) abort
+ let t = self._to_set(a:t)
+ return self.len() > t.len() && self.issuperset(t)
+endfunction
+
+function! s:_base_set.len() abort
+ return len(self._data)
+endfunction
+
+function! s:_base_set.to_list() abort
+ return values(self._data)
+endfunction
+
+function! s:_base_set._update(xs) abort
+ if s:_is_set(a:xs)
+ call self.update(a:xs.to_list())
@thinca
thinca Dec 7, 2014 Member

update() は base set には存在しないように見えます。

@haya14busa
haya14busa Dec 7, 2014 Member

なんでテスト落ちなかったんだろう... ありがとうございます...!

@haya14busa
Member

Set でも重要なファクターの1つ、元にした Python のでいうところの x in s がない予感がします…!

ハッ...! 実装します

@thinca
Member
thinca commented Dec 7, 2014

Python の set って pop() あるのか…。斬新すぎる…!

@thinca thinca and 1 other commented on an outdated diff Dec 7, 2014
autoload/vital/__latest__/Data/Set.vim
+function! s:_base_set.to_list() abort
+ return values(self._data)
+endfunction
+
+function! s:_base_set._update(xs) abort
+ if s:_is_set(a:xs)
+ call self.update(a:xs.to_list())
+ return
+ endif
+ for X in a:xs
+ call self._add(X)
+ unlet X | endfor
+endfunction
+
+function! s:_base_set._add(x) abort
+ let self._data[string(a:x)] = a:x
@thinca
thinca Dec 7, 2014 Member

一意性のために string() を使うと、例えばネスト構造のある辞書などが使えなくなりますが、その点について理解していますか?

@haya14busa
haya14busa Dec 7, 2014 Member

うー確かに...

!!!TypeError: unhashable type: 'dict'!!!

pythonだと dictlistに対してはそもそも受け入れてないですね.(ならそもそも弾けという話ではある)

つまりこれを実装したら解決しそうデスかね #239 (comment)

@thinca
thinca Dec 7, 2014 Member

ハッシュ関数と言う形であればそうなります。
しかし毎回ハッシュ関数を用意するのは手間なので、デフォルトでよしなにする方法はやはり欲しいですね…。
ちなみに Python の場合は dict かどうかとかは関係なくて、hashable かどうかで決まります。

@haya14busa
haya14busa Dec 7, 2014 Member

しかし毎回ハッシュ関数を用意するのは手間なので、デフォルトでよしなにする方法はやはり欲しいですね…。

デフォルトの関数は string() + dict や listはそもそも弾くにしておくとかはどうでしょう. あー...というかどちらにせよ辞書を使ってる実装をまず変えないとだめっぽいかな

@thinca thinca commented on the diff Dec 7, 2014
doc/vital-data-set.txt
+
+Set.gt({other}) *Vital.Data.Set-Set.gt()*
+ Test whether the set is a proper superset of other, that is, >
+ set.ge(other) && set != other
+
+Set.len() *Vital.Data.Set-Set.len()*
+ Return the length of Set
+
+------------------------------------------------------------------------------
+Mutable Set Object *Vital.Data.Set-Set-mutable*
+
+ The following methods for set do not apply to immutable instances of
+ |Vital.Data.Set.frozen()|
+
+Set.update({other}) *Vital.Data.Set-Set.update()*
+Set.ior({other}) *Vital.Data.Set-Set.ior()*
@thinca
thinca Dec 7, 2014 Member

この辺りの ior とか iand とかの i ってなんでしょう…?

@haya14busa
haya14busa Dec 7, 2014 Member

pythonではこう(set |= other)なるやつですね. iがなんの略かは勉強不足でイマイチわからないです...

@thinca
thinca Dec 7, 2014 Member

Python を参考にするのはとても良いんですが、文化まで丸々持ってくることはない気がします。
i なんちゃらはなくてもいいかなってのが個人的な意見です。(まあ update() がわかりづらいってのは別にあるかもしれないですが)

@haya14busa
haya14busa Dec 7, 2014 Member

確かに要らないかな〜とも思ってたんですが andorに対応したミュータブル版という意味で名前はともかくあってもいいかなと思うので悩ましいデス...

iじゃなくていい感じの名前をつければいいかなー

@thinca
thinca Dec 7, 2014 Member

いい感じの名前が付けられるならそれが一番良さそうです(思い付かないけど…)。

@thinca
thinca Dec 7, 2014 Member

今実装見てたんですが、ドキュメント上だと両者は同じものになっていますが、実装上だと微妙に違いますね。ドキュメント通り Set を渡している分には同じなので問題はないかな…?

@haya14busa
haya14busa Dec 7, 2014 Member

一応違いを書いてたんですが書いてない部分もあったのでドキュメントは書き足しました.
haya14busa@432e2e9

@haya14busa
haya14busa Dec 7, 2014 Member

というかリストも受け取れて, set オブジェクトも返してくれる両者のいいとこ取りをするように1つにまとめればいい気がしてきた

@thinca
Member
thinca commented Dec 7, 2014

コミットメッセージに Data.Set がついてませんよ。

@haya14busa
Member

コミットはあとでrebaseしまする

@thinca
Member
thinca commented Dec 7, 2014

合点承知

@haya14busa
Member

あーでもむしろまとめないほうが良い...!という場合はprefixつけておきます

@thinca thinca commented on an outdated diff Dec 7, 2014
doc/vital-data-set.txt
+
+Set.difference_update({other}) *Vital.Data.Set-Set.difference_update()*
+Set.isub({other}) *Vital.Data.Set-Set.isub()*
+ Update the set, removing elements found in {other}.
+
+Set.symmetric_difference_update({other})
+ *Vital.Data.Set-Set.symmetric_difference_update()*
+Set.ixor({other}) *Vital.Data.Set-Set.ixor()*
+ Update the set, keeping only elements found in either set, but not in
+ both.
+
+Set.add({elem}) *Vital.Data.Set-Set.add()*
+ Add {elem} to the set.
+
+Set.remove({elem}) *Vital.Data.Set-Set.remove()*
+ Remove {elem} from the set. Throw |E716| if {elem} is not
@thinca
thinca Dec 7, 2014 Member

例外が飛ぶのわかっているなら専用の例外を用意した方が良さそうです。
そうすることで意図した例外なのかバグによる例外なのか区別できます。

@thinca thinca commented on the diff Dec 7, 2014
autoload/vital/__latest__/Data/Set.vim
+ endif
+ endfor
+endfunction
+
+" Remove all elements of another set from this set.
+function! s:set.isub(t) abort
+ call self._binary_sanity_check(a:t)
+ call self.difference_update(a:t)
+ return self
+endfunction
+
+" Remove all elements of another set from this set.
+function! s:set.difference_update(t) abort
+ let t = self._to_set(a:t)
+ if self is t
+ call self.clear()
@thinca thinca commented on an outdated diff Dec 7, 2014
autoload/vital/__latest__/Data/Set.vim
+ unlet X
+ endfor
+endfunction
+
+function! s:_base_set._add(x) abort
+ let self._data[string(a:x)] = a:x
+endfunction
+
+"" Report whether an element is a member of a set.
+function! s:_base_set.__contains__(e) abort
+ return index(self.to_list(), a:e) > -1
+endfunction
+let s:_base_set.in = s:_base_set.__contains__
+
+" Check that the other argument to a binary operation is also a set, raising a
+" TypeError otherwise.
@thinca
thinca Dec 7, 2014 Member

TypeError とは。

@haya14busa
Member

すぐじゃなくていいと思いますが、一意性を定義するための関数 (恐らくハッシュ値を生成するものか、同一性の比較関数) が与えられると便利そうです。

function! s:_base_set._new(...) abort
  let obj = deepcopy(self)
  let xs = a:0 > 1 ? a:000 : get(a:, 1, [])
  let obj._is_addable = get(a:, 2, function('s:_is_addable'))
  call obj._set_data(xs)
  return obj
endfunction
" ...
function! s:_is_addable(xs, x) abort
  return index(a:xs, a:x) is -1
endfunction

ハッシュ値生成つらそうだったので, こういう感じにしようと思うのですがよいですかね...? あーリストとじゃなくて, やっぱり要素同士を比較するふうにしたほうがよいかな

@haya14busa
Member

こっちかな...?

function! s:_is_identical(x, y) abort
  return a:x is a:y
endfunction
@haya14busa
Member

OrderedSet はデフォルト string() で指定できるようにしてた https://github.com/vim-jp/vital.vim/blob/master/autoload/vital/__latest__/Data/OrderedSet.vim#L14

@thinca
Member
thinca commented Dec 7, 2014

要素同士の比較の方が良さそうですね。
ただその場合も2通りあって、1つは同値かどうかをチェックするもの。もう1つは大小関係をチェックするもの。
関数を用意する側としては前者の方が手軽だけど、追加や削除に O(n) のオーダーが必要になります。
後者の場合はツリー構造やバイナリサーチを使うことで O(log n) にすることができます。

@thinca
Member
thinca commented Dec 7, 2014

OrderedSet、その関数の指定方法が help に載ってないですね…。ping @tyru

@haya14busa
Member

んー大小関係をチェックするほうの場合, funcref, list, dict あたりを要素に使えなくなりそうですね

@thinca
Member
thinca commented Dec 7, 2014

デフォルトは用意できませんね。

@haya14busa
Member

O(log n) は魅力的ですがfuncrefとか対応したいという思いがあるので単に同値かチェックするようにしました

@haya14busa
Member

古いびむ, function()さんにスクリプトローカル関数渡してもよしなにしてくれないのつらすぎる

@haya14busa
Member

Data.List の uniq, どうしてるのかなーと思ったら普通に string() だった

@haya14busa
Member

あーその前に f適用してるからいいのか

@haya14busa
Member

func_alias, まだ使えないんだった. これがマッジされたら入れようかな #240

@haya14busa
Member

Set と FrozenSet を混ぜて使った場合の戻り値について説明が欲しいです。

かきました haya14busa@683e77f

@haya14busa
Member

やっぱり比較関数だとめっちゃ遅かったこと, uniq_by とか OrderdSet も辞書使ってたのでやっぱり実装はハッシュ値生成にしました. ネスト構造ある辞書はどうしようかな...

@thinca
Member
thinca commented Dec 10, 2014

themis 1.4 マージしたよっ!

@haya14busa
Member

themis 1.4 マッジベンリっ...!

@thinca
Member
thinca commented Mar 11, 2015

ping

@ujihisa
Member
ujihisa commented Mar 15, 2015

いよいよマージすると聞いて

@haya14busa
Member

遅くなってスイマセンっ 🙇
rebase doneデス

@haya14busa
Member

複数の引数に対応する場合 

はとりあえず一旦保留でもよいですかね? 実装するには関数専用に追加するか可変長引数になってどっちでもウーンという感じがないでもない

@thinca thinca merged commit 10af898 into vim-jp:master Mar 22, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@thinca
Member
thinca commented Mar 22, 2015

マージしてしまいました!🍣
追加で実装したければいつでも welcome 🎁

@haya14busa
Member

マッジありがとうございますっ
追加実装したくなったら/必要になったらまたPRします

@haya14busa haya14busa deleted the haya14busa:add-set-module branch Mar 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment