galaxy icon indicating copy to clipboard operation
galaxy copied to clipboard

Fix counterintuitive behavior of has_n_cols for csv

Open d-callan opened this issue 1 month ago • 6 comments

basically encountered counterintuitive behavior where has_n_columns assertion in tests for a tool that output csv kept failing saying it only saw 1 column in the file. delimiter apparently defaults to tab for csv ftype. apparently i can explicitly set sep for csv to make this work, which is great, outside the fact i had no idea i needed to. id rather not need to, so this pr represents a proposal to change the delimiter based on ftype.

How to test the changes?

(Select all options that apply)

  • [x] I've included appropriate automated tests.
  • [ ] This is a refactoring of components with existing test coverage.
  • [ ] Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • [x] I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

d-callan avatar Dec 04 '25 03:12 d-callan

I'm on the fence on this, I fear that having a default changing with the datatype could be more confusing than helpful. The sep attribute is clearly documented at https://docs.galaxyproject.org/en/master/dev/schema.html#id228

nsoranzo avatar Dec 04 '25 16:12 nsoranzo

I'm on the fence on this,

Then a profile version could be an idea.

bernt-matthias avatar Dec 04 '25 18:12 bernt-matthias

Then a profile version could be an idea.

i think i understand this sentence, and i think i like it lol

d-callan avatar Dec 04 '25 19:12 d-callan

fwiw looking through the other tabular data types, they appear at glance to all be tab delimited except csv. so the existing default works except for csv, in which case its highly confusing and feels like a bug.

d-callan avatar Dec 04 '25 23:12 d-callan

ok. well, ive updated the docs i was pointed to. and ive taken a look around and think i see how to make the profile version thing work. so let me know if thats something worth pursuing.

d-callan avatar Dec 05 '25 02:12 d-callan

so let me know if thats something worth pursuing.

IMO Yes.

bernt-matthias avatar Dec 05 '25 09:12 bernt-matthias