Issue#45 refactoring script and test

hrsano645 commented 3 years ago

👏 解決する issue / Resolved Issues

close #45

📝 関連する issue / Related Issues

43
42

⛏ 変更内容 / Details of Changes

大規模なリファクタリングを行いました。
オープンデータの入力とdata.jsonのそれぞれのデータの生成部分を関数にして分離しました
それに伴い関数のテストを追加しました
リファクタリングなので、生成結果は以前の状態と変わりません。
- https://github.com/hrsano645/covid19-gen-datajson-shizuokapref/tree/dad95af99af58fd1d8d997e0b8daeb7129a93889 のcommitでデータの生成をテストして同様の結果となることを確認済みです。

📸 スクリーンショット / Screenshots

aktnk commented 3 years ago

差分では確認しにくかったので、動作フローを作成して関数名と動作の整合性を中心に確認してみました。下記に作成した動作フロー図を掲載します。

全体動作フロー
gen_querents()の動作フロー
gen_patient()の動作フロー
gen_patient_summary()の動作フロー
gen_inspections_summary()の動作フロー
gen_main_summary()の動作フロー

hrsano645 commented 3 years ago

@aktnk 確認ありがとうございます。（フロー図すごいです。これって何か自動で生成させていますか？）もう少し、処理の中身を手入れしたほうがいいところもあるのですが（例えばgen_関数でNoneを返すのに考慮してない、関数でカプセル化した後での変数名の整理）、ひとまず目的は達成できたと思ってます。レビューの内容を修正したうえでmergeしようと思います。

修正については以下を行います。

（日付のバリデーション処理と日付の生成を分ける -> ） バリデーションするタイミングをオープンデータ生成時にして、問題があったときにエラーとして落とすようにする
- https://github.com/hrsano645/covid19-gen-datajson-shizuokapref/pull/46#discussion_r584242840
gen_datelist関数にテストを追加する
- https://github.com/hrsano645/covid19-gen-datajson-shizuokapref/pull/46#discussion_r584242849
change_datetimestr_zeropadding関数の名前を suppress_zero に変更
- https://github.com/hrsano645/covid19-gen-datajson-shizuokapref/pull/46#discussion_r584242864

aktnk commented 3 years ago

（フロー図すごいです。これって何か自動で生成させていますか？）

よく言えば、途中から自動生成です。実装コードを読んで、PlantUML用のコードに自分が書いて、フロー図はPlantUMLに生成させています。より詳しく説明すると、１）PlantUMLサーバはDockerで立ち上げて、２）VSCode＋PlantUML Pluginを使いPlantUMLサーバと連携させ、３）自分が書いたUMLのアクティビティ図のコードをプレビューしたり、PNGファイルを生成させています。

hrsano645 commented 3 years ago

@aktnk の指摘を反映しました。変更のコミットとコメントを残しておきます。

（日付のバリデーション処理と日付の生成を分ける -> ） バリデーションするタイミングをオープンデータ生成時にして、問題があったときにエラーとして落とすようにする

#46 (comment)

-> https://github.com/hrsano645/covid19-gen-datajson-shizuokapref/commit/7273e0581305506d83a7bdfb34f0f38dc8e87b17

日付のバリデート関数(isvalid_opendata_dateformat)とバリデート関数を各項目に実行する関数(validate_dataset関数）を追加しました。
main関数でバリデーションの処理を追加して、バリデートしてエラーがある文字列をデータ名とセットに出力して例外終了するようにしました。
data.jsonの各データの処理で入れてあったバリデート処理を外しました。

gen_datelist関数にテストを追加する

#46 (comment)

-> https://github.com/hrsano645/covid19-gen-datajson-shizuokapref/commit/7fc30970969df0d84065b5aa381a7dc0f1cdeefe

gen_datelist関数にテストを追加しました。
テストはstartとendの範囲とその範囲の日数を見ています。

change_datetimestr_zeropadding関数の名前を suppress_zero に変更

#46 (comment)

-> https://github.com/hrsano645/covid19-gen-datajson-shizuokapref/commit/c78abb8122d8d2bcea1ee631e9e4d4d27a0bde9f

hrsano645 commented 3 years ago

レビューコメントにも返信で変更内容を追加しました。

hrsano645 commented 3 years ago

バリデーションがどのように機能するか説明します。

テストデータを用意して実行した様子です。二つのファイルを一部の行だけ残して、日付をエラーが起こるようにわざと変更しています。

test_number.csv

実施_年月日,全国地方公共団体コード,都道府県名,"検査実施_人数
（保健所）","検査実施_人数
（医療機関等）",備考
～2020/4/26,220001,静岡県,"2,637",828,累計用
2020/4/27,220001,静岡県,39,63,
/2020/4/28,220001,静岡県,83,73,
2020/4/29,220001,静岡県,63,38,
~2020/4/30,220001,静岡県,38,32,
220/5/1,220001,静岡県,97,57,

call_center.csv

受付_年月日,全国地方公共団体コード,都道府県名,市区町村名,相談件数
2020/2/10,220001,静岡県,,0
/2020/2/11,220001,静岡県,,0
2020/2/12,220001,静岡県,,47
2020/2/13,220001,静岡県,,49
~2020/2/14,220001,静岡県,,74
2020/2/15,220001,静岡県,,61
2020/2/16,220001,静岡県,,28
220/2/17,220001,静岡県,,164
2020/2/18,220001,静岡県,,199

定義された日付のフォーマットではない場合は以下のように表示されます。

patients.pyの実行結果

Traceback (most recent call last):
  File "patients.py", line 621, in <module>
    main()
  File "patients.py", line 553, in main
    raise DateValidateError(error_msg)
__main__.DateValidateError: バリデーションの結果、処理出来ない行があります。処理を終了します。:
call_center: /2020/2/11
call_center: ~2020/2/14
call_center: 220/2/17
inspections_summary: /2020/4/28
inspections_summary: ~2020/4/30
inspections_summary: 220/5/1

hrsano645 commented 3 years ago

@aktnk ありがとうございます！mergeしますね。

（参考） patients.pyですが、各csvに対して似たような処理が繰り返される構造なので、クラス化することで、凝集度を高めかつ、結合度を下げることができるかもしれないと思いました。

クラス化も考えてはいましたが、まずは現状の1ファイルのみでコードの状況整理をしたかったので、いきなりやらないでおきました。

このスクリプトもスクリプトといえる行数でもないので、モジュール分離をして各市町対応しやすい仕組みにできたらいいと思います。

hrsano645 / covid19-gen-datajson-shizuokapref