Skip to content

Conversation

fnks-tk
Copy link

@fnks-tk fnks-tk commented Jul 17, 2024

  • Excelをcsvに変換するOfficeスクリプト
  • 変換されたcsvをOpenAPI Specification(JSON)に変換するスクリプト
  • サンプルのExcelファイル
    を含みます。

今後の方向性として考えているのは、pandasでExcelを入力できるようなので、Officeスクリプトを使わずにすべてPythonで完結させることを考えています。
pd_read_excel.pyはExcelの特定シートを読み込むスクリプトです。

@fnks-tk fnks-tk requested a review from yuki-js July 17, 2024 08:11
Copy link
Member

@yuki-js yuki-js left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[ほめ] 56グリッドシステム、JTCらしさが出ていていいですね。しっかり再現していてわかりやすいです。一般的には2と3を素因数に持っていてほしいのですが、まあもっと大事なこと考えてない故の方眼紙なので、これも味としましょう。
[必須] C13:C14 が空欄です。

image

[ノート]全体的に何を狙ったコードなのかが分からないです。コードを見る限り /examplePath に 一つ get200 を生やした程度のJSONファイルを出力しているように見えます。ベンダー Xxxxxxxxx 社とか XXX 社の形式が悪いといえばそれまでなのですが、OpenAPIのゴールと照らし合わせれば、これの存在意義はないと解さざるを得ません。GitHubは要件含む仕様以上の問題を解決する機能はないので、今後一緒に詰めていけたらと存じます。

}
else:
nested_property = {
row['項目(英名)']: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[質問]
Sample.xlsx.csvを読み取らせたとき、下記のようなエラーが表示されました。キーがなくなる原因、または正しいファイルを頂けますか?内部で回して頂いて構いません。

KeyError: '項目(英名)'

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こちらの環境でもcsvファイルによって発生することがあり疑問だったのですが、確認したところ、このJSONへの変換プログラムはOfficeスクリプトでcsvを整形せずに出力する場合で試した際のプログラムでした。今回のpushにはcsvを整形して出力するようなOfficeスクリプトと、そのスクリプトで生成したcsvファイルをサンプルとして格納しているため、実行がうまくいかなかったのだと思います。期待した結果通り実行できるcsvファイルをDiscordでお送りします。
ただ、csvを読み取る方式から変更しようとしているため実装として変わる可能性が高いです。

@@ -0,0 +1,6 @@
No,階層,項目名(日本語),項目名(英語),データ型,桁数,必須,入出力区分,種別,項目の説明,値の例
1,1,コンテンツタイプ,content-type,string,y,入力,HEADER,レスポンスヘッダーのコンテンツタイプ,application/json;charset=utf-8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[推奨]CSVのカラム数が一致しません。一部の実装系で正しく処理できず、表示ができないので、目視で対応するのが大変だなと思います。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Officeスクリプトを使ってcsv化した際に、Excel方眼紙に対して実行すると一部項目が抜けてしまう(うまく変換できない)ことは理解しており、その点pandasを使えばわざわざExcelをcsvに変換せずにpandas.DataFrameとしてうまく処理ができそうということもあり、Pythonだけでの実装を現在進めています。

Copy link
Member

@yuki-js yuki-js left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

第二弾


for row in reader:
hierarchy = row['階層'].split('.')
if row['項目(英名)'] == 'Content-Type':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[推奨]カラム「種別区分」を適切に使用して処理をしていただきたいです
[推奨]Content-Typeはヘッダには定義しないでいただきたいです。仕様上は合っていますが、それを含めるとヘッダを出力するようなコードをバックエンド側で書かなければならないと、自動生成コードの都合上なってしまいます。レスポンスMIMEタイプはレスポンスボディの方で定義に入れてください。

reader = csv.DictReader(file)
components_schemas = {}
path_specs = {
"/examplePath": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[必須]パス名を変更できるようにしてください
[必須]複数のパス名とメソッドの組を定義してください

path_specs = {
"/examplePath": {
"get": {
"summary": "Example summary",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[必須]operationIdを記載してください。CSVカラムも追加してください。

}

current_level[next_level]["properties"].update(properties)
return current_level
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[任意]current_levelがdictなら、参照渡しになるので、in-placeでアップデートする関数であるとした方が私は好みだな

if not hierarchy:
return properties

next_level = hierarchy.pop(0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[質問] hierarchyはどんな変数ですか?ドキュメンテーションを行うことで回答に代えてもよいです。

else:
nested_property = {
row['項目(英名)']: {
"type": "string" if row['データ型'] == 'string' else "integer",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[必須]objectやarrayにも対応してください。

Copy link
Member

@yuki-js yuki-js left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

第二弾

@fnks-tk
Copy link
Author

fnks-tk commented Jul 18, 2024

@yuki-js
Excel to yamlのスクリプトを追加しました。出力されるyamlは意図と異なるため再度修正を行います。
Pythonスクリプト中にてコメントアウトで理想のOASの出力結果について書いたので、誤っている認識があれば指摘してほしいです。

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants