search

andreicuceu / vega

GNU General Public License v3.0
9 stars 5 forks source link

improved error reporting #136

Open iprafols opened 8 months ago

iprafols commented 8 months ago

In some cases, if there is a negative redshift in the catalogue (i.e. due to problems in redshift estimation), the metal code will fail. This does not solve the issue, but rather informs the user of the specific nature of the problem, so that they can go ahead and fix their catalogue

codecov[bot] commented 8 months ago

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 41.47%. Comparing base (275d061) to head (3302f3c).

Files Patch % Lines
vega/metals.py 0.00% 7 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #136 +/- ## ========================================== - Coverage 41.53% 41.47% -0.06% ========================================== Files 27 27 Lines 3527 3532 +5 Branches 701 702 +1 ========================================== Hits 1465 1465 - Misses 1917 1922 +5 Partials 145 145 ``` | [Flag](https://app.codecov.io/gh/andreicuceu/vega/pull/136/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrei+Cuceu) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/andreicuceu/vega/pull/136/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrei+Cuceu) | `41.47% <0.00%> (-0.06%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrei+Cuceu#carryforward-flags-in-the-pull-request-comment) to find out more.

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

calumgordon commented 3 weeks ago

@iprafols I guess this isn't urgent (the e.g. Y3 BAO catalogue doesn't have negative redshifts), do you still want to introduce this feature?

andreicuceu commented 3 weeks ago

Hi @calumgordon, @iprafols, long term it would be great to move this to picca, so we don't need to deal with QSO catalogs here. In short term, this fix should work. Did anyone test that it works, and does not slow down the code?