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

Suppress misleading RASCIL warning #603

Closed mpluess closed 2 weeks ago

mpluess commented 2 months ago

Suppress misleading RASCIL warning "The RASCIL data directory is not available - continuing but any simulations will fail" which confused users. RASCIL simulations work perfectly fine without the data directory, to the best of our knowledge. Closes #615

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.

Project coverage is 68.62%. Comparing base (457a68e) to head (8e9ba23). Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
karabo/simulation/signal/seg_u_net_segmentation.py 0.00% 1 Missing :warning:
karabo/util/rascil_util.py 92.85% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #603 +/- ## ========================================== + Coverage 68.61% 68.62% +0.01% ========================================== Files 54 53 -1 Lines 5690 5696 +6 ========================================== + Hits 3904 3909 +5 - Misses 1786 1787 +1 ```

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

anawas commented 1 month ago

Shall I review this PR or do you squash it yourself? I f you want me to approve it please add me as a reviewer.

mpluess commented 1 month ago

Shall I review this PR or do you squash it yourself? I f you want me to approve it please add me as a reviewer.

Thanks for the offer. It would be great if you can help with PR reviewing in general. However, for this specific PR, there's no need. I discussed this topic with Lukas and would like for him to have a look at the changes when he's back to make sure they don't break anything.

PHerzogFHNW commented 1 month ago

I added tests confirming that

On a more general note though, I do also feel uneasy about referencing a class by file path. Even if our test succeeds, if a user somehow ends up with a different path to their installation_checks.py (maybe it moves in a future version of rascil and they updated their rascil library) it's gonna break, and it would be very difficult to figure out why.