search

necst-telescope / necst

:gem: NEw Control System for Telescope
https://necst-telescope.github.io/necst/
MIT License
3 stars 0 forks source link

#181 channel binning #221

Closed FumikaDemachi closed 10 months ago

FumikaDemachi commented 1 year ago

Checks

Related Issue

This PR closes #181 .

codecov[bot] commented 1 year ago

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (fbde8aa) 71.73% compared to head (73946d8) 71.61%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #221 +/- ## ========================================== - Coverage 71.73% 71.61% -0.12% ========================================== Files 59 59 Lines 3106 3125 +19 ========================================== + Hits 2228 2238 +10 - Misses 878 887 +9 ``` | [Files](https://app.codecov.io/gh/necst-telescope/necst/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=necst-telescope) | Coverage Δ | | |---|---|---| | [necst/definitions.py](https://app.codecov.io/gh/necst-telescope/necst/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=necst-telescope#diff-bmVjc3QvZGVmaW5pdGlvbnMucHk=) | `100.00% <100.00%> (ø)` | | | [necst/rx/spectrometer.py](https://app.codecov.io/gh/necst-telescope/necst/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=necst-telescope#diff-bmVjc3Qvcngvc3BlY3Ryb21ldGVyLnB5) | `23.12% <33.33%> (-0.10%)` | :arrow_down: | | [necst/core/commander.py](https://app.codecov.io/gh/necst-telescope/necst/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=necst-telescope#diff-bmVjc3QvY29yZS9jb21tYW5kZXIucHk=) | `53.31% <0.00%> (-1.09%)` | :arrow_down: | | [necst/procedures/observations/observation\_base.py](https://app.codecov.io/gh/necst-telescope/necst/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=necst-telescope#diff-bmVjc3QvcHJvY2VkdXJlcy9vYnNlcnZhdGlvbnMvb2JzZXJ2YXRpb25fYmFzZS5weQ==) | `30.64% <14.28%> (-0.98%)` | :arrow_down: | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/necst-telescope/necst/pull/221/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=necst-telescope)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

FumikaDemachi commented 10 months ago

@TatsumiISHIKAWA 確認ありがとうございます。

  1. 動作試験の際にQ-lookに渡されるdata列の長さを確認し、その結果を見る限りは問題なさそうに思いました。
  2. 日報にメモしたように、chを与えない場合はobservation_base.pyのexecute()の中で条件分岐して処理しています。つまりchを指定しない場合は分光計のデフォルトの値のままです。
TatsumiISHIKAWA commented 10 months ago

@FumikaDemachi 了解です。

  1. Topicとしては問題なさそうなのですね。個人的に受け取られた分光データのチャンネル数が変わった時にObserverの表示が追いつくのか？という不安があるのですが、necst, neclibの動作としては大丈夫そうですので実際に使ってみて問題が出たら対処することにしますか...
  2. 確認しました。argparseの使用上実装が難しかったことを思い出しました...追加で申し訳ないのですが、各観測モードのbinのheaderについているコマンドの使い方にChannel Binningの要素を追加をお願いします。（ついでにhot_monitorのコマンド使用例が実は間違っているという不具合があるので直していただけると助かります。）

それとはこれと別の問題ですが、もしかしてResizeの改修をしたことによって何箇所かに散りばめられているQ-Lookのチャンネル数の初期化は必ずしも必要じゃなかったりしませんか...？