uribo / zipangu

Japanese utility functions and data
https://uribo.github.io/zipangu
Other
56 stars 7 forks source link

Unify the output of `kansuji2arabic_all` with the other output #24

Closed indenkun closed 4 years ago

indenkun commented 4 years ago

既存のkansuji2arabic_allは複数の文字列を与えたときに、結果出力時に文字列が結合されてしまいます。

zipangu::kansuji2arabic_all(c("一", "二", "三"))
[1] "123"
> zipangu::kansuji2arabic_all(c("一歳", "二年", "三月"))
[1] "1歳2年3月"

kansuji2arabicでは

zipangu::kansuji2arabic(c("一", "二", "三"))
[1] "1" "2" "3"

となるので、特別な意図がないのであれば同様の挙動をしてほしいと思っています。

もとのコードで行っている処理を最大限維持するのであれば、purrr:mapで処理するのが妥当かと考えており、そのコードを提案します。

これで

zipangu::kansuji2arabic_all(c("一", "二", "三"))
[1] "1" "2" "3"
zipangu::kansuji2arabic_all(c("一歳", "二年", "三月"))
[1] "1歳" "2年" "3月"
zipangu::kansuji2arabic_all(c("一歳", "二年", "三月"), .under = 2)
[1] "1歳"  "二年" "三月"

となるはずです。

些末な提案ですみませんが、ご検討ください。

uribo commented 4 years ago

良い提案ですね。ありがとうございます。 少しコメントしました。

indenkun commented 4 years ago

コメントの通り、たしかにkansuji2arabicのようにconvert = FALSEとしても数値型にならないので、purrr::map_chr()のほうがコードがスッキリすると思いました。 修正します。

indenkun commented 4 years ago

purrr::map()からpurrr::map_chr()に変更しました。

indenkun commented 4 years ago

ありがとうございます。