JASMINE-Mission / jasmine-imagesim

image simulator of JASMINE
0 stars 0 forks source link

Feature/drift params20220731 #97

Closed tkamizuka closed 2 years ago

tkamizuka commented 2 years ago

drift.json のフォーマットを他の json とあわせ、説明書きを追記。 この変更にあわせて extract_json も修正。 ご確認のほど、よろしくおねがいいたします。

astronasutarou commented 2 years ago

……ところで init=False のせいで現在まともに動いていない状態でしょうか?自分の環境でも extract_json 関数が以下のエラーを吐くことを確認しました.

AttributeError: 'Drift' object has no attribute 'drift_time'
tkamizuka commented 2 years ago
  • params の他のディレクトリに drift.json の兄弟がいるのでそちらの変更もお願いします.

この兄弟って何者なのでしょうか。今後も継続サポートを必要としますでしょうか。必要であればアップデートします。必要ないのであればメンテ負担軽減で削除しようとも思いますが、ご確認いただけますでしょうか。

  • Drift のコメントのアップデートも一緒に含めていただけるとありがたいです.

これ何でしたっけ。何か漏れていたということでしょうか。読解力が不足していてすみません…。

tkamizuka commented 2 years ago

……ところで init=False のせいで現在まともに動いていない状態でしょうか?自分の環境でも extract_json 関数が以下のエラーを吐くことを確認しました.

こちらですが、関数単体を実行するとエラーを出しますが、a=Drift(hogehoge) みたいにして使うとエラーを出しません。とりあえずそれで動くので仕様と思ってスルーしましたが…。

tkamizuka commented 2 years ago

……ところで init=False のせいで現在まともに動いていない状態でしょうか?自分の環境でも extract_json 関数が以下のエラーを吐くことを確認しました.

こちらですが、関数単体を実行するとエラーを出しますが、a=Drift(hogehoge) みたいにして使うとエラーを出しません。とりあえずそれで動くので仕様と思ってスルーしましたが…。

いろいろみていると皆さん default オプション使うか __post_init__ 使うかして何かしらの値が入るようにして使っているような感を持ちましたが、そんなルールを明記したものには当たれていません…。

astronasutarou commented 2 years ago

兄弟は正直いないほうが良いと思っています……動かない version が同居しているのもよくないですし特に使われていないようであれば削除したいところはあります.

Drift のコメントのアップデート

Drift クラスの定義直後に書いてあるコメントで Drift velocity in ???. とかいうところです!

astronasutarou commented 2 years ago

なんとなくですが a = Drift(arguments) はオッケーで Drift(arguments) はエラーを吐くという挙動はちょっとイヤらしい (放置しておきたくない) 気はするのですよね…….

tkamizuka commented 2 years ago

兄弟は正直いないほうが良いと思っています……動かない version が同居しているのもよくないですし特に使われていないようであれば削除したいところはあります.

名前的に兄弟を作ったのは @HajimeKawahara さんな気がするので河原さんにご確認いただきましょう。

Drift のコメントのアップデート

Drift クラスの定義直後に書いてあるコメントで Drift velocity in ???. とかいうところです!

了解しました。

tkamizuka commented 2 years ago

なんとなくですが a = Drift(arguments) はオッケーで Drift(arguments) はエラーを吐くという挙動はちょっとイヤらしい (放置しておきたくない) 気はするのですよね…….

DefaultでNone入れときますか?

astronasutarou commented 2 years ago

下手な数値を入れるよりは None を入れておいたほうが良さそうですね.賛成します.

tkamizuka commented 2 years ago

@xr0038 Drift のコメントの件と default 値の方修正しましたのでご確認ください。

@HajimeKawahara params dir. にある exoflat, planet というのは今後もサポートすべきものなのかご確認ください。

tkamizuka commented 2 years ago

動作試験さぼってすみません。 そのままでは動かなかったのですが、動くように改良しました。 引き続きよろしくお願いいたします @xr0038 @HajimeKawahara

astronasutarou commented 2 years ago

惑星用は何らかの形式ではサポートが必要ですね、、、

かわはらさんのコメント on Slack

astronasutarou commented 2 years ago

サポートのやり方として params/template のコピーを作るのはあまり筋が良くないと思っています.とはいえちゃんと「サポート」するには仕組みを整える必要があるので,とりあえず今回は書き換えた drift.json を兄弟に分け与えて,処する方法を考える (issue 行き?) という方針でどうでしょうか? @tkamizuka

codeclimate[bot] commented 2 years ago

There are too many results to compare

View more on Code Climate.

tkamizuka commented 2 years ago

了解しました。対応しまし…codeclimate が引っかかりました。

tkamizuka commented 2 years ago

が、jis への codeclimate は追々だったと思うので、review のほど何卒よろしくお願いいたします。 @HajimeKawahara

astronasutarou commented 2 years ago

すみません……とりあえず設定してみただけなので Code Climate の check は無視してください……

tkamizuka commented 2 years ago

@HajimeKawahara さま。お忙しいところ恐縮ですが、review のほうよろしくお願いいたします。