Skip to content

Minor change to get proper output quoting in writeJannoFileWithoutEmptyCols#393

Merged
nevrome merged 2 commits into
masterfrom
fixedQuotingInWriteJannoWithoutEmptyCols
Jul 11, 2026
Merged

Minor change to get proper output quoting in writeJannoFileWithoutEmptyCols#393
nevrome merged 2 commits into
masterfrom
fixedQuotingInWriteJannoWithoutEmptyCols

Conversation

@nevrome

@nevrome nevrome commented Jul 9, 2026

Copy link
Copy Markdown
Member

I noticed an issue in the output quoting of writeJannoFileWithoutEmptyCols. It caused the output of trident rectify --jannoRemoveEmpty to be unreadable to trident! The minimal change in this PR does seem to solve the problem.

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 52.03%. Comparing base (7ba3f31) to head (82bc0e8).

Files with missing lines Patch % Lines
src/Poseidon/Core/Janno.hs 75.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #393   +/-   ##
=======================================
  Coverage   52.03%   52.03%           
=======================================
  Files          37       37           
  Lines        5994     5994           
  Branches      644      644           
=======================================
  Hits         3119     3119           
  Misses       2231     2231           
  Partials      644      644           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@stschiff

Copy link
Copy Markdown
Member

OK, I think I understand the change, looks OK to me. I'm a bit surprised about the back and forth here... why do we first encode the Janno into byte string, then decode again, filter for non-empty columns using transpose and lists, and then encode back again. Isn't that one loop too many? Why can't we just do the filter immediately on the Janno Rows? Is that due to the columns perhaps not being aligned and uniform? Probably, right?

@nevrome

nevrome commented Jul 11, 2026

Copy link
Copy Markdown
Member Author

In their normal state the janno rows have all sorts of types and it's complicated to determine if they are NA or not. After encoding every NA value is a string n/a. So the easiest way to check if an entire column is NA is to encode, then decode to Bytestring. It's really just a hack, but it works and does the thing in a few lines of code.

@nevrome nevrome merged commit 5aef90d into master Jul 11, 2026
4 checks passed
@nevrome nevrome deleted the fixedQuotingInWriteJannoWithoutEmptyCols branch July 11, 2026 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants