save all values comma separated instead of only the last one #6
Conversation
|
The failing CI build is unrelated to my changes and results from the use of the Deprecated Series.irow method |
|
Hey @moritzschaefer I just noticed this PR -- is it valid to have a key occur more than once? And when that happens, are you sure that appending the values into a string representation of comma-separated list is the only/best action? |
|
The |
|
@moritzschaefer I think this issue #2 runs into the same problem as you but their suggested solution is keeping a list of options. That seems safer to me than a comma separated string -- what do you think of changing this PR to keep lists of multi-values? |
|
OK, just implement the list-accumulating version of this PR here: #8 Curious whether this works for @moritzschaefer |
|
Gencode gtf has keys used more than once a lot. This does not mean, it is valid, though it means it is the case. The reason I used a comma-separated list, is that when using read_gtf_as_dataframe, I wanted to have a single value field at each position in the DataFrame (for performance reasons). I think it's also feasible to convert the list objects to strings after read_gtf_as_dataframe is performed. Did you test your PR on real data? (https://github.com/openvax/gtfparse/pull/8/files) |
|
@moritzschaefer The str vs. list representation seems to be a memory vs. runtime tradeoff. It's cheaper to make lists but I think will use more memory than a concatenated string. I'm going with lists for now since it feels cleaner. I just added a unit test in this PR: #9 (as well as fixing deploy, so hopefully this will actually go to PyPI now) |
3 participants
Right now only the last value is returned in the DF if a given attribute key exists more than once in one GTF-line. It doesn't make sense to just return that last value, so I changed the code such that a comma-separeted string is returned for these attributes instead.