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

Data.Counter: Implement "Data.Counter" #348

Merged
merged 12 commits into from Dec 28, 2015

Conversation

Projects
None yet
4 participants
@haya14busa
Member

haya14busa commented Dec 7, 2015

Data.Counter is similar to python's collections.Counter

I'll add document later but the implementation is done and I assume you can understand most of behavior from comments, tests and python's documentation of collections.Counter.
Please take a look when you have time.

@haya14busa haya14busa changed the title from Data.Counter: Implement "Data.Counter" [WIP for document] to Data.Counter: Implement "Data.Counter" Dec 9, 2015

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Dec 9, 2015

Member

ドキュメント足しました (めんどいので日本語)

Member

haya14busa commented Dec 9, 2015

ドキュメント足しました (めんどいので日本語)

Show outdated Hide outdated doc/vital-data-counter.txt
echo sort(s:c.elements())
" => ['A', 'A', 'B', 'B', 'C', 'C']
<
Counter.most_common({number}) *Vital.Data.Counter-Counter.elements()*

This comment has been minimized.

@crazymaster

crazymaster Dec 9, 2015

Member

s/elements()/most_common()/

@crazymaster

crazymaster Dec 9, 2015

Member

s/elements()/most_common()/

This comment has been minimized.

@haya14busa

haya14busa Dec 9, 2015

Member

fixed

@haya14busa
@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Dec 9, 2015

Member

正直微妙なところ

pythonのcollections.Counterの+-はnegative valueを無視します.
現状のPRのVim script 実装では+, -に対応するメソッドは実装せず,
negative valueを無視しない.update(), .subtract() を実装しています.

これは返ってきたCounterにたいしてpythonでは+c, このVim
script版では.keep_positive()すれば十分じゃないか?というのと,
-を実装しようとするとsubtract(sub)はすでにあって使えないので名前がムズイ...という理由です.
+.add()でokなんですが...

.update()の仕様はpythonの仕様と同じになってるけど.add()のほうがわかりやすそう.
しかし.add()だとpythonにならってnegativeを無視したほうがいいのでは...?となるので実装してない.

Member

haya14busa commented Dec 9, 2015

正直微妙なところ

pythonのcollections.Counterの+-はnegative valueを無視します.
現状のPRのVim script 実装では+, -に対応するメソッドは実装せず,
negative valueを無視しない.update(), .subtract() を実装しています.

これは返ってきたCounterにたいしてpythonでは+c, このVim
script版では.keep_positive()すれば十分じゃないか?というのと,
-を実装しようとするとsubtract(sub)はすでにあって使えないので名前がムズイ...という理由です.
+.add()でokなんですが...

.update()の仕様はpythonの仕様と同じになってるけど.add()のほうがわかりやすそう.
しかし.add()だとpythonにならってnegativeを無視したほうがいいのでは...?となるので実装してない.

Show outdated Hide outdated autoload/vital/__latest__/Data/Counter.vim
return result
endfunction
" .to_list() returns list of element in the counter. The order is arbitary.

This comment has been minimized.

@crazymaster

crazymaster Dec 9, 2015

Member

s/arbitary/arbitrary/

@crazymaster

crazymaster Dec 9, 2015

Member

s/arbitary/arbitrary/

This comment has been minimized.

@haya14busa

haya14busa Dec 9, 2015

Member

ありがとうございます.修正ed

@haya14busa

haya14busa Dec 9, 2015

Member

ありがとうございます.修正ed

Show outdated Hide outdated autoload/vital/__latest__/Data/Counter.vim
return map(values(self._dict), 'v:val.value')
endfunction
" .values() returns list of count in the counter. The order is arbitary.

This comment has been minimized.

@crazymaster

crazymaster Dec 9, 2015

Member

s/arbitary/arbitrary/

@crazymaster

crazymaster Dec 9, 2015

Member

s/arbitary/arbitrary/

Show outdated Hide outdated doc/vital-data-counter.txt
" => {'A': 2, 'B': 2, 'C': 2}
<
Counter.to_list() *Vital.Data.Counter-Counter.to_list()*
Returns list of element in the counter. The order is arbitary.

This comment has been minimized.

@crazymaster

crazymaster Dec 9, 2015

Member

s/arbitary/arbitrary/

@crazymaster

crazymaster Dec 9, 2015

Member

s/arbitary/arbitrary/

Show outdated Hide outdated doc/vital-data-counter.txt
" => ['A', 'B', 'C']
<
Counter.values() *Vital.Data.Counter-Counter.values()*
Returns list of count in the counter. The order is arbitary.

This comment has been minimized.

@crazymaster

crazymaster Dec 9, 2015

Member

s/arbitary/arbitrary/

@crazymaster

crazymaster Dec 9, 2015

Member

s/arbitary/arbitrary/

Show outdated Hide outdated autoload/vital/__latest__/Data/Counter.vim
\ : a:countable
endfunction
" ._hash() returns hasy key for given value.

This comment has been minimized.

@crazymaster

crazymaster Dec 9, 2015

Member

s/hasy/hash/

@crazymaster

crazymaster Dec 9, 2015

Member

s/hasy/hash/

Show outdated Hide outdated autoload/vital/__latest__/Data/Counter.vim
" s:new() creates a new instance of Counter object.
" @param {list|string|dict?} countable (optional)
function! s:new(...) abort
let c = deepcopy(s:Counter)

This comment has been minimized.

@thinca

thinca Dec 9, 2015

Member

引数チェックの後で良さそう。

@thinca

thinca Dec 9, 2015

Member

引数チェックの後で良さそう。

This comment has been minimized.

@haya14busa

haya14busa Dec 19, 2015

Member

たしかにっ

@haya14busa

haya14busa Dec 19, 2015

Member

たしかにっ

Show outdated Hide outdated doc/vital-data-counter.txt
------------------------------------------------------------------------------
FUNCTIONS *Vital.Data.Counter-functions*
new({countable}) *Vital.Data.Counter.new()*

This comment has been minimized.

@thinca

thinca Dec 9, 2015

Member

引数省略可の場合は [{countable}] で。

@thinca

thinca Dec 9, 2015

Member

引数省略可の場合は [{countable}] で。

This comment has been minimized.

@haya14busa

haya14busa Dec 19, 2015

Member

fixed

@haya14busa
@thinca

This comment has been minimized.

Show comment
Hide comment
@thinca

thinca Dec 9, 2015

Member

.update() は上書きされる印象がありますね。個人的には .add() の方がわかりやすいかなと思います。(Python のことは忘れる…)

Member

thinca commented Dec 9, 2015

.update() は上書きされる印象がありますね。個人的には .add() の方がわかりやすいかなと思います。(Python のことは忘れる…)

Show outdated Hide outdated doc/vital-data-counter.txt
echo s:c.in('A')
" => 1
echo s:c.in('D')
" => 1

This comment has been minimized.

@lambdalisue
@lambdalisue

This comment has been minimized.

@haya14busa

haya14busa Dec 19, 2015

Member

thanks for the catch!

@haya14busa

haya14busa Dec 19, 2015

Member

thanks for the catch!

@lambdalisue

This comment has been minimized.

Show comment
Hide comment
@lambdalisue

lambdalisue Dec 11, 2015

Member

Python のことを忘れて add に一票

Member

lambdalisue commented Dec 11, 2015

Python のことを忘れて add に一票

@lambdalisue

This comment has been minimized.

Show comment
Hide comment
@lambdalisue

lambdalisue Dec 11, 2015

Member

-c.keep_negative() にする(非破壊・反転)でいかが?
あと

add --- subtract     : マイナス値を考慮
increase -- decrease : マイナス値を排除

などは?

Member

lambdalisue commented Dec 11, 2015

-c.keep_negative() にする(非破壊・反転)でいかが?
あと

add --- subtract     : マイナス値を考慮
increase -- decrease : マイナス値を排除

などは?

@thinca

This comment has been minimized.

Show comment
Hide comment
@thinca

thinca Dec 19, 2015

Member

ping

Member

thinca commented Dec 19, 2015

ping

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Dec 19, 2015

Member

ちょっと放置しててすいませんでした 🙇

  • s/update/add/g しました
  • -c の存在完全にわすれてましたが .keep_positiveは破壊的なのでやるとしても破壊的に実装するかkeep_positiveごと変更ですかね.非破壊の場合なんとなくkeepはわかりずらい感もある
  • increase/decreaseはなんとなく英語的に違和感ある気がするけどよくわからない力. 引数が増えるのではなくself(主語的な?)が増えるわけだし
Member

haya14busa commented Dec 19, 2015

ちょっと放置しててすいませんでした 🙇

  • s/update/add/g しました
  • -c の存在完全にわすれてましたが .keep_positiveは破壊的なのでやるとしても破壊的に実装するかkeep_positiveごと変更ですかね.非破壊の場合なんとなくkeepはわかりずらい感もある
  • increase/decreaseはなんとなく英語的に違和感ある気がするけどよくわからない力. 引数が増えるのではなくself(主語的な?)が増えるわけだし
@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Dec 19, 2015

Member

-cの件は符号を反転するのが面倒なのが問題だと思ったので.reverse()足しました.
0以下を削除したい場合は.keep_positive()と組み合わせればok

Member

haya14busa commented Dec 19, 2015

-cの件は符号を反転するのが面倒なのが問題だと思ったので.reverse()足しました.
0以下を削除したい場合は.keep_positive()と組み合わせればok

@thinca

This comment has been minimized.

Show comment
Hide comment
@thinca

thinca Dec 19, 2015

Member

Assert Equals() の引数の順序が軒並み逆と言う衝撃の事実。

Member

thinca commented Dec 19, 2015

Assert Equals() の引数の順序が軒並み逆と言う衝撃の事実。

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Dec 20, 2015

Member

ハッ...バレタ...というか思い出した...!!!!

書いてる途中で逆なことに気づいたんですが最後に機械的に反転させようとおもったんでした.そして忘れてた 💦

原因としては Data.List のテストの最初の方が実は逆になってるっぽくて参考にしてしまってミスったので別PRかなんかでなおそうかと思います

Member

haya14busa commented Dec 20, 2015

ハッ...バレタ...というか思い出した...!!!!

書いてる途中で逆なことに気づいたんですが最後に機械的に反転させようとおもったんでした.そして忘れてた 💦

原因としては Data.List のテストの最初の方が実は逆になってるっぽくて参考にしてしまってミスったので別PRかなんかでなおそうかと思います

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Dec 20, 2015

Member

引数の順序修正ed

Member

haya14busa commented Dec 20, 2015

引数の順序修正ed

@thinca

This comment has been minimized.

Show comment
Hide comment
@thinca

thinca Dec 20, 2015

Member

ちょー細かいんですが、各テストのタイトルの最後に . があったりなかったりするのが気になりました (統一するならない方に一票)

Member

thinca commented Dec 20, 2015

ちょー細かいんですが、各テストのタイトルの最後に . があったりなかったりするのが気になりました (統一するならない方に一票)

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Dec 20, 2015

Member

ふぃっくすed & ついでにrebased

Member

haya14busa commented Dec 20, 2015

ふぃっくすed & ついでにrebased

Show outdated Hide outdated test/Data/Counter.vimspec
Assert Equals(c.get('l'), 1)
End
It override count

This comment has been minimized.

@thinca

thinca Dec 20, 2015

Member

overwrites かな?

@thinca

thinca Dec 20, 2015

Member

overwrites かな?

Show outdated Hide outdated test/Data/Counter.vimspec
Assert Equals(c.to_dict(), {'function(''tr'')': 2, 'function(''function'')': 1})
End
It add a new counter from a dictionary

This comment has been minimized.

@thinca

thinca Dec 20, 2015

Member

adds

@thinca
Show outdated Hide outdated test/Data/Counter.vimspec
Assert Equals(c.to_dict(), {'A': 2, 'B': -1, 'C': 0})
End
It add a new counter from a counter

This comment has been minimized.

@thinca

thinca Dec 20, 2015

Member

adds

@thinca
Show outdated Hide outdated test/Data/Counter.vimspec
Assert Equals(c1.union(c2).to_dict(), {'a': 1})
End
It should handle dictionary as an argument

This comment has been minimized.

@thinca

thinca Dec 20, 2015

Member

should いらなそう。

@thinca

thinca Dec 20, 2015

Member

should いらなそう。

Show outdated Hide outdated test/Data/Counter.vimspec
End
End
Describe elements()

This comment has been minimized.

@thinca

thinca Dec 20, 2015

Member

.elements()

@thinca

thinca Dec 20, 2015

Member

.elements()

@thinca

This comment has been minimized.

Show comment
Hide comment
@thinca

thinca Dec 20, 2015

Member

操作系の機能のほとんどは破壊的っぽいけど、.union().intersection() だけは非破壊かな? だとしたらその説明とテストがあるとベンリそう。

Member

thinca commented Dec 20, 2015

操作系の機能のほとんどは破壊的っぽいけど、.union().intersection() だけは非破壊かな? だとしたらその説明とテストがあるとベンリそう。

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Dec 20, 2015

Member

たしかにっ! 修正ed

Member

haya14busa commented Dec 20, 2015

たしかにっ! 修正ed

@thinca

This comment has been minimized.

Show comment
Hide comment
@thinca

thinca Dec 20, 2015

Member

完全に LGTM 👍 🍣

Member

thinca commented Dec 20, 2015

完全に LGTM 👍 🍣

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Dec 20, 2015

Member

アッ最後のcommitいらんto_dictあるので消します

Member

haya14busa commented Dec 20, 2015

アッ最後のcommitいらんto_dictあるので消します

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Dec 20, 2015

Member

fixed

Member

haya14busa commented Dec 20, 2015

fixed

thinca added a commit that referenced this pull request Dec 28, 2015

Merge pull request #348 from haya14busa/data-counter
Data.Counter: Implement "Data.Counter"

@thinca thinca merged commit b9b7f8e into vim-jp:master Dec 28, 2015

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@haya14busa haya14busa deleted the haya14busa:data-counter branch Dec 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment