poseidon-framework / poseidon-hs

A toolset to work with modular genotype databases in the Poseidon format
https://poseidon-framework.github.io/#/trident
MIT License
7 stars 2 forks source link

Again allowed 0 in the Nr_SNPs .janno column #316

Closed nevrome closed 3 weeks ago

nevrome commented 3 weeks ago

This should solve https://github.com/poseidon-framework/poseidon-schema/issues/85

codecov[bot] commented 3 weeks ago

Codecov Report

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

Project coverage is 59.99%. Comparing base (d4d4e7f) to head (dcd910c). Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
src/Poseidon/ColumnTypes.hs 0.00% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #316 +/- ## ======================================= Coverage 59.99% 59.99% ======================================= Files 29 29 Lines 4179 4179 Branches 481 481 ======================================= Hits 2507 2507 Misses 1191 1191 Partials 481 481 ```

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

stschiff commented 3 weeks ago

Hmm, this is OK, but different from what I thought we discussed in https://github.com/poseidon-framework/poseidon-schema/issues/85. I thought we wanted to still issue a warning if the NrSNPs is 0. Now we only get an error if it's negative. That's quite different. Is that on purpose?

nevrome commented 3 weeks ago

Puh - I didn't consider the warning important, honestly. The problem is that the packages with Nr_SNPs < 1 don't get read right now.

Warnings require separate code, because make only lives in MonadFail. I can add it :man_shrugging:.

stschiff commented 3 weeks ago

Ah OK. So you want to keep the error-behavior, but restricted to truly false cases (negative numbers). I think that's a good solution then!