全対象URLをローカルで機械学習し、質問する前に選別する #204
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/arakawatomonori/covid19-surveyor/3r8jm1495 |
| continue | ||
| fi | ||
| echo "$md5,$text" >> ./data/eval.csv | ||
| done < ./data/urls-md5.csv |
There was a problem hiding this comment.
urls-md5.csvにある4万件のURLをすべてwgetして、テキストを抽出して、機械学習できるcsvに整える
There was a problem hiding this comment.
入力データの urls-md5.csv の最後のほうが壊れているように見えるのは既出でしたっけ。
この PR に限った話ではないですが、各 .csv の各列が何を示しているか分かる情報が無いとコードレビューにも少し支障をきたすかな、というお気持ちがあります。
たとえば .csv であれば1行目に見出しとして各列の意味情報を入れておく（その上で .csv を読み取るプログラムは1行目の読み込みはスキップする）と分かりやすいです。このファイルの場合は hash,url という構造なのだな、ということは「雰囲気で」分かるのですが。
| @@ -146,7 +163,7 @@ main() { | |||
| #members_list="xUUL8QC8BUx xU011H85CM0Wx xUUQ99JY5Rx xU011C3YGDABx" | |||
| for member in $members_list; do | |||
| member_id=${member:1:-1} | |||
| send_message $member_id | |||
| echo `send_message $member_id` | |||
There was a problem hiding this comment.
いまさらなのだけれど send_message という名前が分かりにくかった。
「なんでこれ引数がひとつなんだ？ send_message(member_id, message) じゃないの？」と思って、実装を読みに行ったら send_question(_from_queue) みたいな内容だった。
という、レビューしました。
There was a problem hiding this comment.
send_message がなんでもやりすぎ問題。
send_message が関数名から予想できない動きになっているのをどうにかしたい。
send_message() {
member_id = $1
message = $2
# ... メッセージ送信
}
send_question() {
member_id = $1
question = get_question
send_message($member_id, $question)
}
みたいに分けて、 send_question の呼び出しになっているとよさそう。
There was a problem hiding this comment.
change を suggest したい気はするのだけれど、動作環境や動かし方のドキュメント or テストがないので、動くコードを書けない。
| ### 機械学習で評価した結果とURLのmd5を対応付けたファイルを生成する | ||
| ### | ||
| data/eval-results-md5.csv: tmp/eval-result.csv | ||
| cat tmp/eval.csv|cut -d',' -f 1 > tmp/md5.csv |
There was a problem hiding this comment.
tmp/eval.csv というファイルパスは意図通りですか？
他の eval.csv は data/eval.csv となっているようですが。
| # result.txtからURLのみを抜き出す | ||
| urls=$(cat ./tmp/results.txt | cut -d':' -f 1 | sed -z 's/\.\/www-data\///g') | ||
| urls=$(cat ./tmp/grep-aggregate.txt | cut -d':' -f 1 | sed -z 's/\.\/www-data\///g') |
There was a problem hiding this comment.
今回の変更で tmp/results.txt が tmp/grep-aggregate.txt にリネームされたという解釈で合ってますか？
解釈が合っている前提でコメントすると、
# result.txtからURLのみを抜き出すというコメント文も書き換えてほしいです。（要望）tmp/grep-aggregate.txtというファイル自体はどこから出現する（生成される）ものなのでしょうか？（質問）.dockerignoreファイル内に書かれている/result.txtという行も追随して変更しなくて大丈夫ですか？（疑問）
|
Note |
|
#209 マージされました。 👍 |
No description provided.