Skip to content

Fix archiver bug ignoring deletions when comparing two files with no missing columns #1522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 2, 2022

Conversation

dshemetov
Copy link
Contributor

@dshemetov dshemetov commented Feb 9, 2022

Description

A partial fix of the issues in #1520.

Changelog

Previously, the archiver did not code deletions between two old-style CSVs (old-style meaning no missing columns). This changes it to always write deletions with DELETED missing columns and NAs in the missing columns for the rest of the non-missing rows. The validation and repair for the NA missing columns is then handled by the acquisition.

To be merged after cmu-delphi/delphi-epidata#862.

Fixes

@dshemetov dshemetov requested a review from krivard February 9, 2022 23:30
@dshemetov dshemetov changed the title Fix archiver bug treating old deletions as new deletions, add many tests Fix archiver bug ignoring deletions when comparing two files with no missing columns Feb 28, 2022
@dshemetov dshemetov requested a review from krivard March 1, 2022 18:06
@krivard krivard mentioned this pull request Mar 2, 2022
Copy link
Contributor

@krivard krivard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

legit!

I weep for your time spent juggling numbered csv files -- I threw together #1542 as a followon (nonurgent) so nobody ever has to do that again

@krivard krivard merged commit 6b5263d into main Mar 2, 2022
@krivard krivard deleted the dshem/fix-archiver branch March 2, 2022 18:13
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.

Repair initial issue for Quidel:Omicron edition
2 participants