i4Ds / Karabo-Pipeline

The Karabo Pipeline can be used as Digital Twin for SKA
https://i4ds.github.io/Karabo-Pipeline/
MIT License
11 stars 4 forks source link

Fix issue #581 #602

Closed anawas closed 1 month ago

anawas commented 2 months ago
codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 68.61%. Comparing base (c1aed2c) to head (597c889).

Files Patch % Lines
karabo/simulation/signal/seg_u_net_segmentation.py 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #602 +/- ## ========================================== - Coverage 68.65% 68.61% -0.05% ========================================== Files 54 54 Lines 5689 5690 +1 ========================================== - Hits 3906 3904 -2 - Misses 1783 1786 +3 ```

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

Lukas113 commented 2 months ago

I'll wait for the review until #601 is merged since #601 is also in this PR.

mpluess commented 2 months ago

@anawas can you merge main into this branch and solve the conflicts? i will review it after that's done.

anawas commented 2 months ago

@mpluess Done, tests successful

anawas commented 2 months ago

Could do it for OSKAR telescopes. However, plotting a RASCIL instrument works differently. The function Telescope::plt_telescope() can not create a file for RASCIL. The call in the RASCIL package is:

plot_configuration(config, ax=None, plot_file=None, title='Configuration', label=False, **kwargs)

Long story short: I have to rewritte the function in class Telescope.

mpluess commented 1 month ago

Changes look good so far, tests are failing though

anawas commented 1 month ago

That's what I talked about on Slack. I did not touch the file the pre-commit hook is complaining about. I ran the tests locally and they passed. This pre-commit hook is bit of hard to deal with. Screenshot 2024-08-02 115004

mpluess commented 1 month ago

That's what I talked about on Slack. I did not touch the file the pre-commit hook is complaining about. I ran the tests locally and they passed. This pre-commit hook is bit of hard to deal with. Screenshot 2024-08-02 115004

I seems hard to have 100 % the same environment as the CI tests on GitHub. I think all of us devs have had to fix things in reaction to tests failing on GitHub before. Had to remove the same "type: ignore" comment in another branch this morning. One difference is probably that on GitHub, a new environment is created on every action, meaning our dev envs are probably a bit behind on same package versions. Updating the environment more often could open.