search

monarch-initiative / koza

Data transformation framework for LinkML data models
https://koza.monarchinitiative.org/
BSD 3-Clause "New" or "Revised" License
48 stars 4 forks source link

If skip_blank_lines is False in CSVReader, garbage data is read out in all cases #158

Open ptgolden opened 5 days ago

ptgolden commented 5 days ago

The second clause here should be elif not row or elif row == [].

https://github.com/monarch-initiative/koza/blob/fceafe5755e222dba14cecf8b7df09ca7dd16831/src/koza/io/reader/csv_reader.py#L126-L130

Since it's not, if skip_blank_lines=False is passed to the reader, then in all cases, every value read will be 'NaN' for str types, nan for float types, and raise an error for int types.

I'm surprised no one has ever run into this-- it suggests that no one has ever used the non-default option of skip_blank_lines.

ptgolden commented 5 days ago

It looks like that clause was introduced in 80cf06858072a5a53fd0330f06d632373eda58ae, which means 3 years and no reports of output consisting solely of "NaN"

ptgolden commented 4 days ago

skip_blank_lines is never set to false, because it's not passed to the CSVReader constructor when reading a source:

https://github.com/monarch-initiative/koza/blob/fceafe5755e222dba14cecf8b7df09ca7dd16831/src/koza/model/source.py#L38-L48

Can we remove this option? Either blank lines can always be skipped (which is the current behavior), or they can cause an error (due to being malformed CSV).