Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

入力ファイルの制限なし #23

Merged
merged 4 commits into from
Jun 10, 2024
Merged

入力ファイルの制限なし #23

merged 4 commits into from
Jun 10, 2024

Conversation

KeiTa4446
Copy link
Member

@KeiTa4446 KeiTa4446 commented Jun 7, 2024

#22 のマージが前提

Close #17

Description(変更内容)

  • 入力ファイルを制限するために拡張子を列挙していたが,基本版ではワイルドカードで対応することにした。
  • .tif以外の拡張子に対応する,という意味で,問題がなければ#17は閉じたい。

...

Manual Testing(手動テスト)

  • test.tif以外のDEMデータを入力ファイルに設定して,CS立体図が作成されるか確認してください。

Summary by CodeRabbit

  • 新機能

    • 新しいフィルタとストレージモードをファイルウィジェットに追加。
    • 処理後にウィンドウを閉じるオプションのチェックボックスを追加。
  • バグ修正

    • 入力と出力のファイルハンドリングを改善。
    • エラーメッセージとエラーハンドリングを強化。
  • スタイル

    • UIのウィジェット名とサイズを調整。

Copy link

coderabbitai bot commented Jun 7, 2024

Walkthrough

dem_to_csmap.pydem_to_csmap.uiのスクリプトは、ファイル入出力の処理、エラーハンドリング、フィルターとストレージモードの追加などの機能強化が行われました。DEMからCSMapへの変換ロジックが改善され、エラーメッセージとQGISでの出力処理が向上しました。

Changes

ファイル 変更内容の要約
dem_to_csmap.py QGISコアモジュールのインポート、ファイルウィジェットのフィルターとストレージモードの変更、エラーハンドリングの追加、出力ラスターレイヤーのQGISへの追加。
dem_to_csmap.ui ウィジェットの名前変更、チェックボックスの追加、矩形の高さ変更、ボタンウィジェットのプロパティ追加。

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI
    participant Script

    User->>UI: ファイルを選択
    UI->>Script: 入力パスを取得
    Script->>Script: DEMをCSMapに変換
    Script->>Script: エラーチェック
    Script->>UI: エラーメッセージ(エラー時)
    Script->>UI: 出力パスを取得
    Script->>UI: 出力ラスターレイヤーをQGISに追加
    UI->>User: 処理完了メッセージ
Loading

Poem

変換の道は険しくとも、
新たなフィルターで楽々と、
エラーも捕えて安心さ、
QGISに輝くレイヤーが、
うさぎも跳ねる嬉しさよ。
🐇✨


Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between cbc549c and a5d878b.

Files selected for processing (2)
  • dem_to_csmap.py (2 hunks)
  • dem_to_csmap.ui (2 hunks)
Files skipped from review due to trivial changes (1)
  • dem_to_csmap.ui
Additional context used
Path-based instructions (1)
dem_to_csmap.py (1)

Pattern **/*.py: - can be available more immutable approach?

Additional comments not posted (6)
dem_to_csmap.py (6)

4-5: 新しいインポートが追加されました。QgsFileWidget はファイルウィジェットの設定に使用されています。


26-27: 出力ファイルウィジェットは .tif ファイルのみを受け入れ、ストレージモードは SaveFile に設定されています。出力が .tif のみである理由をコメントで説明することをお勧めします。


37-38: ファイルパスはUIウィジェットから正しく取得され、処理関数に渡されています。ファイルパスの取得に関するエラーハンドリングを追加することをお勧めします。


40-49: 処理中の例外を捕捉するために try-except ブロックが使用されています。より具体的なエラーメッセージを提供することをお勧めします。


52-52: 処理後に出力ラスターレイヤーが QGIS に追加されます。QGIS でのレイヤー追加を確認してください。


23-23: 入力ファイルウィジェットのフィルターがすべてのファイルを受け入れるように設定されています。異なるファイルタイプでの動作を確認してください。

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a5d878b and cfebb29.

Files selected for processing (1)
  • dem_to_csmap.py (3 hunks)
Additional context used
Path-based instructions (1)
dem_to_csmap.py (1)

Pattern **/*.py: - can be available more immutable approach?

Additional comments not posted (4)
dem_to_csmap.py (4)

4-4: 新しいライブラリのインポートが適切に行われています。


22-23: 入力ファイルのフィルターを全てのファイルタイプに設定することで、より柔軟なファイル入力が可能になります。


37-37: 入力ファイルと出力ファイルのパスを適切に取得しています。


51-51: 処理後に出力ラスターレイヤーをQGISに追加する処理が適切に行われています。

dem_to_csmap.py Outdated Show resolved Hide resolved
@geogra-geogra
Copy link
Member

geogra-geogra commented Jun 7, 2024

@KeiTa4446

手動テストについての質問です

test.tif以外のDEMデータ

tifファイル以外で作成されたDEMファイルという意味で合ってますか?
上森さんがテストしたサンプルはありますか?

先日も提案させていただきましたが、他の人がテストしやすいように各種ファイルをSampleディレクトリのようなものを作ってまとめておくと良いと思います。
もちろんそれ以外の形式や違うデータでの検証も必要ですが、まずは上森さんが意図する動作が他の端末でも行えるのかが大切だと思います

@KeiTa4446
Copy link
Member Author

tifファイル以外で作成されたDEMファイルという意味で合ってますか?

上記で間違いないです。
アドバイス通りにサンプルディレクトリを作ったところ,Lintテストでエラーが出たので,使用したファイルをPR時に添付するようにします。
sample.zip

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between cfebb29 and 5ffba5c.

Files selected for processing (2)
  • dem_to_csmap.py (3 hunks)
  • dem_to_csmap.ui (2 hunks)
Files skipped from review due to trivial changes (1)
  • dem_to_csmap.ui
Additional context used
Path-based instructions (1)
dem_to_csmap.py (1)

Pattern **/*.py: - can be available more immutable approach?

Additional comments not posted (2)
dem_to_csmap.py (2)

4-4: インポートされた Qgis は適切に使用されています。


37-37: 入力ファイルパスの取得方法が更新されています。新しいUIコンポーネントの使用に問題はありません。

dem_to_csmap.py Show resolved Hide resolved
dem_to_csmap.py Outdated Show resolved Hide resolved
dem_to_csmap.py Show resolved Hide resolved
@geogra-geogra
Copy link
Member

山本LGTMです
うさぎさんに反応してあげてください(現在必要ない機能には、こういう理由で今それは考えなくていいです、って返信しておけば次回以降そこにツッコミ入れないようにしてくれます)
@Kanahiro さんお願いします

@KeiTa4446
Copy link
Member Author

パラメータを設定した際にプレビューを見られるような機能を足すissueがありますが,それまでは,処理終了後にウィンドウを自動で閉じるかどうか,ユーザーが判断できるようにチェックボックスを追加しました。
以下のテストデータを使用して,チェックボックスの動作を確認していただきたいです。
sample.zip

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5ffba5c and 42bfc87.

Files selected for processing (2)
  • dem_to_csmap.py (3 hunks)
  • dem_to_csmap.ui (3 hunks)
Files skipped from review as they are similar to previous changes (1)
  • dem_to_csmap.ui
Additional context used
Path-based instructions (1)
dem_to_csmap.py (1)

Pattern **/*.py: - can be available more immutable approach?

dem_to_csmap.py Show resolved Hide resolved
dem_to_csmap.py Show resolved Hide resolved
Copy link
Member

@Kanahiro Kanahiro left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -35,17 +34,29 @@ def convert_dem_to_csmap(self):
params = process.CsmapParams()

# 入力・出力をUIで操作
input_path = self.ui.mQgsFileWidget.filePath()
input_path = self.ui.mQgsFileWidget_input.filePath()
Copy link
Member

Choose a reason for hiding this comment

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

nits:

変数名に、キャメルケースとスネークケースは混ぜない方がよいです。
コーディング規約になんか書いてあったかなと読み返すと、書いてなかった。。。
Pythonでは一般的にスネークケースが使われます。ただし、QtのAPIはキャメルケースなので、それらに関する変数名はキャメルケースでも構いません。

@Kanahiro Kanahiro merged commit 16c5cb1 into main Jun 10, 2024
3 checks passed
@Kanahiro Kanahiro deleted the feature/input-wildcard branch June 10, 2024 07:06
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.

.tif以外の拡張子に対応する
3 participants