概要

私はデータデザイン事業部門のエンジニアチームのリーダーをしています。
ここ一年ぐらいの取り組みでデータサイエンスチームのレビューが大分いい感じに回るようになってきたので、そこに至るまでにやってきた諸々を備忘録も兼ねてまとめました。

※機械学習モデルの品質管理などの話は出てきません。あくまでデータ分析プロセスにおけるコードに関する内容になります。

チーム状況・背景

私が所属しているチームは、データの分析や機械学習モデルの開発などデータサイエンスを担当するメンバーと、データ活用基盤やモデルの実行環境を構築するデータエンジニアリング担当のメンバーが混在しています。
明確なチームなどの切り分けはないですし両面やる場合もありますが、どちらかをメインとしているメンバーが多いです。
ちなみに私はシステム畑出身で、担当も主にデータエンジニアリングの人間です。

データエンジニアリング領域では自動テストやp-rベースのレビューなどは当然としてやっていましたが、データサイエンス領域はプロセスや環境が違い、システム開発ライクなプラクティスを正としてそのまま適用するのが難しい面がいくつかありました。
また、データサイエンス担当のメンバーはアカデミックでデータサイエンスを専攻して入社してきた若手が中心で、チーム開発やシステムの開発運用も未経験のため、テストやレビューが大事ということが頭では分かっていても実感として腹落ちしていないメンバーが多かったように思います。

そんな中で、最初は何も考えずに当然やるものとしてテストを書いてGitHub Flowを回すように呼び掛けていたのですが、なかなかうまく回っていない、実行されていても良くなっている感じがあまりしない状況だったため、原因分析と改善をすることにしました。

ツールとしては、プロジェクト管理はBacklog、コード管理はGitLabを使っています。

課題分析

なぜレビューが上手く回っていないのかを知るために、メンバーへのヒアリングと過去のp-rの観察を行い、原因を整理しました。

JupyterNotebookでの分析

データサイエンスチームではJupyterNotebook上でデータをいじることが多いのですが、これの管理が結構大変。
色々と回避策はあるのですが、出力のオールクリーンが必要だったりPythonファイルへの出力だったりで、煩わしさなく継続して使える方法がなかなかありませんでした。

また、Notebook上で行うデータ分析は試行錯誤の連続でサイクルも非常に早いため、作業に没頭している中で、何かしらのアウトプットがあったタイミングでこまめにCommitするというのは「気を付けよう」だけでは難しいところがあります。

書き捨てのコードが多い

分析が一段落したタイミングでCommitはされるとしても、その結果を確認して次の段階に進むと、試行錯誤の跡が残っているNotebookで続きを書くよりも、関心が切り替わったタイミングで新しいNotebookで仕切り直すことが多いです。
また、意外と再利用できる部分が少なかったりもするので、引き継げないコードやテストの量がそれなりに発生してしまいます。
一度テストを書いておけばその後の動作を担保できるのがシステム開発でテストを書く大きなメリットですが、その恩恵が限定的になり、毎回テストを書くコストもかかるという状況でした。

コードの書き方も、Notebookは使い勝手として上から下にどんどん書き進めやすい作りになっていて、そもそもユニットテストが書きやすいようなコードにないことも多かったです。
それをテストのために整理してテストを書いて、でもあまり再利用されないというのはなかなかコストが高いです。

Gitの使い方やp-rの出し方に問題がある

これは完全に自分の教育不足だったのですが、Commitやp-rの粒度が大きかったり、Commitメッセージが変更したファイル名だけだったり、p-rに確認するために必要な情報が足りなかったりという例が散見されました。
この状態だとレビュアーにエスパー力が求められ、負荷がかなり上がってしまいます。

レビューする時間がない/レビューが遅すぎて進みが悪くなる

これは忙しさによって起こっていた側面と、上記の前提情報の質が良くないことによってレビューに時間がかかっていた側面があるように思います。
また、作業コストが上がるほどそれに対するリターンがないとストレスになりますが、書き捨てのコードが多いの項で書いたとおりシステム開発よりも一度やったことの恩恵を受けにくく、コストリターンのバランスが良くなかったため、それのために待つならさっさと先に進みたいという気持ちは理解できます。

レビューの仕方が分からない/何を指摘していいか分からない

これも教育不足ですが、やらないでいると観点も分かってこないのでレビューするのが怖いままという悪循環でした。

やったこと

これらの課題を解決するため、以下の取り組みを行いました。

Gitプロジェクト勉強会

基礎ができていない状況では何をやっても質が上がらないため、インプットから始めました。
Gitの使い方だけではなく、その周辺のフローやなぜそれをやるのか、GitLabの機能でどのようにそれを実行するのかを資料にまとめて勉強会を実施しました。
同時にレビューの心構えについても触れ、「悪いことを指摘するだけがレビューじゃない」「分からないことを聞くのも立派なレビュー」「いいところは称賛しよう」「レビューの時は自分のことは棚に上げよう（あとでこっそり我が振り直そう）」といった価値観を共有しました。

また、Commitの粒度やメッセージの書き方、p-rに記載すべき事項などもまとめておき、繰り返し確認できるようにしました。
バイトや新しいメンバーが増えがちなチームなこともあり、ドキュメント化することで教えがスケールするようになって後が大変楽になりました。
知見はこまめにドキュメント化するの大事。

Issue/Pull Requestのテンプレート化

勉強会で教えるだけでは実践に結びつかないこともあるので、GitLabやプロジェクト管理で使っているBacklogのIssueやp-rのテンプレートを登録しまくり、意識しなくても目的や手順、完了条件を整理するように仕立てました。
特に完了条件が曖昧だと、何を見るべきかが分かりにくくレビューがしにくいため、チェックリスト化して確認しやすいようにしました。

また、最初はシンプルに始め、慣れてきたプロジェクトからテンプレートをアップデートしていくことで、少しずつ質の高い記述ができるようにしていきました。
最初からゴリゴリに項目があると面食らってしまいますからね。

レビューの整理と対象の整理

インプットをしつつ、テストとレビューは必ずするべきものという前提を一度捨てて、内容を整理することにしました。

そもそも何のためにテストやレビューをするかというと、その結果として開発速度だったりプロダクトの品質だったりを上げる（下げない）ためなので、テスト作成やレビューにかかるコスト以上のリターンがないのであればそれは総合的にスループットを落としていることになります。

そう考えて、レビューの観点を一度細分化しました。

オーナーレビュー：そのコードを取り込むことでプロダクト全体にリスクがないかのチェック
ロジックレビュー：実現したいこと通りに実装されているか、非効率な実装になっていないかのチェック
テストレビュー：テストケースが適切か、その実装が適切かのチェック
リーダビリティレビュー：規約の沿っているか、理解できる読みやすいコードになっているかのチェック
p-rレビュー：レビューするための条件がそろっているかのチェック

設計やリファクタリングに関するレビューはただでさえ難しいうえ、データ分析系のコードではさらにしにくい（必要が無いことも多い）ので、あえて外しました。
また、リファクタはp-rのスコープから外れる指摘が発生する場合もあるので、p-rのレビューからは切り出して別のタイミングで議論するのが妥当だと考えました。

加えて、テストについても結合テスト的な全体でInとOutが合っているかのチェックとユニットテストを切り分け、これらがどういうパターンに効果があるかを考えました。

まず、p-rレビューは必須でやるようにしました。
分かりにくいp-rを頑張って解釈しようとしていたメンバーも多かったようで、p-rに対してもコメントしていいという認識を共有できたのは大きかったように思います。
前述のテンプレートのおかげで指摘もしやすくなりました。

他のレビューはタスクのフェーズや性質によってやるやらないを判断するようにしました。

色々試す段階ではNotebookで書き下されることが多く、再利用性も低いので、ロジックレビューとリーダビリティレビューがされていればOKと判断するようにしました。
リーダビリティについても、前述のとおりリファクタの指摘などは不要で、あくまで何をしているのか分からない部分の指摘なんかが中心になります。
（今後最初から整理されたコードを書くという観点では議論したほうがいいですが、そのコードという意味ではもう使わないのに機能が変わらない変更は不要と判断しました。）

Gitでこまめに差分を管理するのも難しいため、アウトプットのタイミングでNotebookのままレビューを行い、テストも全体で所望の処理ができていることが確認できればOKとしました。
ただし、まとまったアウトプットのタイミングでコード(またはNotebook)は必ず社内GitLabにアップすることを徹底しました。

そういった段階を過ぎて、分析やモデル開発の方向性が明確になったもの、特に長い期間かけて漸進的に実験していくプロジェクトに関しては、できるだけNotebookからPythonファイルに切り出し、設計も意識しながら進めてもらうようにしました。
前処理のユニットテストも書けるところから書いてもらうようにします。
長期のプロジェクトは徐々に再利用されるコードが増えるためテストのコストに対してリターンが大きくなってくるのと、ちょっと前処理や使うカラムを変えての比較実験も増えてくるため、差分とバージョンを管理できないと、アウトプットごとの条件を振り返れないリスクが出てくるためです。
これによりGit管理やテストもできるようになるため、システム開発ライクな手法でもレビュープロセスが機能するようになっていき、オーナーレビュー以外は実施できるようになります。

そして徐々に適用範囲を拡大していくことで、前処理やモデルがFIXしてシステム化するフェーズになっても、大きくプロセスが変わることもなく進めることができますし、ロジック部分は既にテストがあるという状態にすることができます。

この辺りのどのプロジェクトをどうするという判断は、基準を明確にすると逆にやりづらくなる雰囲気があったので、プロジェクトの状況やメンバー、顧客との関係性も見ながら都度判断するようにしています。

書棚の設置と技術書購入

マネージャーとかけあって技術書をそこそこな量購入させてもらいました。
また、総務に空いているロッカーを書棚として使う許可をもらい、だれでも自由に読めるようにしました。
自分はKindle派で、おススメの本があっても物理本を貸せなかったので大変助かりました。

輪読会

有志でClean Codeの輪読会を実施しました。
単純に書かれているノウハウによってコードの品質が上がる効果もありますが、レビューの観点が養われるため、レビュワーとして指摘できる内容が増えます。
また、同じ本を読んでディスカッションすることで共通認識が生まれているので、レビューの際に指摘する心理的ハードルも大きく下がります。
「偉い人が言っている」というのも心強いですね。
（Clean Codeの著者は結構強い思想を持っているようで、内容によって意見が分かれることもありますが、それによってワイワイするのもコミュニケーションとして良かったように思います。）

静的解析ツールの導入

インデントのチェックなどは人間のやることじゃないので、Flake8とblackを使うことをp-rの前提としました。
手作業なので自動化したい。

Slack Botによるレビュアーのランダム指名

忙しいという課題はマネジメントに任せつつ、どうにかしてみんなでレビューを回していくために、Slack Botを使ってレビュアーをランダムに指名するやり方を導入しました。
忙しい人にレビューを依頼しにくいという心理ハードルも下がりますし、レビュー初心者にも一定の割合で振られるので、経験を積んでもらうことができます。
ゲーム性もあり結構好評です。

乱数が偏ってるのか、たまに4回連続で回ってきたりしますが、笑いを提供できるので私は嬉しいです。
~~ロジックに細工されてるんじゃないかとコード見に行ったのは内緒~~

最後に

記事なのでいい感じに書いてはいますが、書いたことが完璧に実践されているわけではないですし、過渡期から続いているプロジェクトなんかは結構厳しい状況から頑張って立て直してたりはします。
ものによっては明文化できておらずに運用でカバーしていたりしますし、実践率50%に届いているか怪しい施策もあります。
ただ、テストやレビューの効果を実感したメンバーが少しずつ増えてきているなとは感じています。

もちろん独力で全部やったわけではないですし、メンバーの理解や頑張りがあっての施策の実現や成果だと思っています。
また、部門全体でカンバンやIssue Templateを取り入れてやっていこうという流れがあり、エンジニアだけでなく部門全体でそういう空気を作ってくれているおかげでやりやすくなった部分も大いにあります。
引き続き、どういうやり方が自分たちにとってやりやすくてリスクヘッジもできるやり方なのかを探りながら、みんなで頑張っていきたいと思います。

明日は213さんの「今年起こした障害の話」です。
本番環境でやらかしちゃった人 Advent Calendar が大変盛りあがっているので、こちらにも期待（？）ですね！