Skip to content

Conversation

@d-callan
Copy link
Contributor

@d-callan d-callan commented Dec 4, 2025

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)

  • 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

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

Copy link
Contributor

@bernt-matthias bernt-matthias left a comment

Choose a reason for hiding this comment

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

Good idea.

Do you think we should cover other tabular datatypes as well:
https://github.com/galaxyproject/galaxy/blob/dev/lib/galaxy/datatypes/tabular.py. Guess with csv we have at least the most frequent one.

Could you update the docs:

<xs:element name="has_n_columns">

@nsoranzo
Copy link
Member

nsoranzo commented Dec 4, 2025

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

@bernt-matthias
Copy link
Contributor

I'm on the fence on this,

Then a profile version could be an idea.

@d-callan
Copy link
Contributor Author

d-callan commented Dec 4, 2025

Then a profile version could be an idea.

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

@d-callan
Copy link
Contributor Author

d-callan commented Dec 4, 2025

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
Copy link
Contributor Author

d-callan commented Dec 5, 2025

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.

@bernt-matthias
Copy link
Contributor

so let me know if thats something worth pursuing.

IMO Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants