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.

Sign up for GitHub

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

Jump to bottom

Add plot scripts #153

Merged
merged 5 commits into from
Jul 19, 2022
Merged

Add plot scripts #153

merged 5 commits into from
Jul 19, 2022

Conversation

200km
Copy link
Member

@200km 200km commented Jun 27, 2022

Overview

Add plot scripts

Issue

Details

Python scripts to plot satellite position and GS visibility analysis on the miller projection map are added.

Validation results

See #146

Scope of influence

Code is not changed

Supplement

NA

Note

NA

@200km 200km added priority::medium priority medium tools labels Jun 27, 2022
@200km 200km requested a review from sksat June 27, 2022 13:56
@200km 200km self-assigned this Jun 27, 2022
@200km
Copy link
Member Author

200km commented Jun 27, 2022

調整したい点

  • ファイルの置き場所
  • 必要なライブラリなどの導入手順をどこに書くか
  • そもそもpython詳しくないので書き方があってるか心配

@sksat
Copy link
Collaborator

sksat commented Jun 28, 2022

必要なライブラリについては，Pythonのもののみであれば(追加でOS側に入れるようなものが無ければ)，依存関係が書かれたファイルが同じディレクトリに転がっていればいいと思います．具体的にはrequirements.txtみたいなやつ(実際はPipfileとかの方が管理などがやりやすいですが，後から移行もできるので一旦必要なものが書いてあればよいです)．

@sksat
Copy link
Collaborator

sksat commented Jun 28, 2022

Pythonはこういうその時一度動けばいいやつに関してはなあなあで書いちゃっていいと思います．
一方で「その時」が気が付いたら来なくなっていると困るので動くかどうかの検証はしておきたくて，そこはCI組んでおいて壊れたら直す，というかんじにはしたいですね(Pythonの場合型検査などでの保証ができないので，基本的には動かしてみるしかない)．
もちろんメンテしやすいコードの書き方，みたいなのはあるわけですが，そこはレビューと(壊れた時に)ちゃんと直すきっかけがあればなんとかなると思います．

sksat
sksat reviewed Jun 28, 2022
View reviewed changes
scripts/Plot/plot_gs_visibility.py Outdated
Comment on lines 8 to 12
# Ground Station Position
gs_lat_deg = 26.140837
gs_lon_deg = 127.661483
# CSC file name
read_file_name = '../../data/SampleSat/logs/logs_220627_142946/220627_142946_default.csv'
Copy link
Collaborator

Choose a reason for hiding this comment

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

ここらへんは実行時にプログラム引数とかから取りたいですね

Copy link
Member Author

Choose a reason for hiding this comment

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

そうですね。何も引数与えなかったら、一番新しいログファイルを取ってくるとかにしたいなとも思ったりしましたが、そういうのは今後実装する~　という感じでいいかなと思ってます。

Copy link
Collaborator

Choose a reason for hiding this comment

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

そうですねー．そこらへんの便利機能は後でいいと思います．

@sksat sksat mentioned this pull request Jun 28, 2022
Merged
4 tasks
@200km
Copy link
Member Author

200km commented Jun 28, 2022

コメントありがとうございます。必要ライブラリなどについて、requirement.txt作っておきます。
CIに入れ込む件もご提案ありがとうございます。よろしくお願いいたします。

@200km 200km changed the title WIP: Add plot scripts Add plot scripts Jul 19, 2022
@200km
Copy link
Member Author

200km commented Jul 19, 2022

@sksat Sorry for my late, but I added the requirements.txt and input arguments.

@200km 200km merged commit aa43248 into develop Jul 19, 2022
@200km 200km deleted the feature/add_plot_scripts branch July 19, 2022 12:25
@200km 200km mentioned this pull request Jul 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sksat sksat sksat approved these changes

Assignees

@200km 200km

Labels
priority::medium priority medium tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@200km @sksat