WIP: Use subjects.csv instead of subjects.json in sortable-table.js

yeatmanlab / AFQ-Browser

Browser-based visualization tools for AFQ results

BSD 3-Clause "New" or "Revised" License

33 stars 15 forks source link

WIP: Use subjects.csv instead of subjects.json in sortable-table.js #187

Closed richford closed 6 years ago

richford commented 6 years ago

Resolves #180.

Remaining issues:

[x] subjectId should be the first column. sortable-table.js preserves the order in the source data so when we were using subjects.json then the first column was subjectId but now that we are using subjectId, the first column has a blank heading and we discard it later. I can make the javascript always use subjectId as the first column and then populate the rest of the table in the order provided?
[x] In this branch, patient_02 had a different number of digits of precision for the score field in the json file than it did in the csv file. This leads to some ugly formatting. Do we want to impose a max number of digits to represent numerical data?

arokem commented 6 years ago

I think that the answer is yes to both. We want subjectID to always be the first column on the left of the table. I don't entirely understand how to change to CSV changed that, but maybe it's not important that I understand?

For the second thing: where did all those digits come from? Either way, I think that for floating point we want maybe 4 digits after the decimal?

richford commented 6 years ago

Both changes are simply from the difference in the actual content of the files. The csv has those digits while the json doesn't. Likewise, the columns are ordered differently in the json than in the csv.

We could fix the csv to be formatted correctly, but I'm thinking that it's better not to require that subjectId be the first column and instead write that into the parsing function in the javascript.

Same for the digits. We could require a certain max precision in the csv files, but it's probably better just to write a formatter in the javascript.

For the precision, I need your domain knowledge @arokem and @jyeatman. Is a user ever likely to need more than, say, nine digits (or six) for any numerical field in the metadata table?

richford commented 6 years ago

Okay, column sorting is fixed so that subjectID is always first. And the formatting is fixed as well. The app will inspect each column of the table data, remove null and undefined values, and put it into one of three format categories:

binary (use d3.format("0b"))
number (use d3.format(".7g"))
string (don't do any extra formatting)

@arokem, @jyeatman, @akeshavan, @anotherjoshsmith Are those the only format categories we need and do those formatting choices look okay?

If so, I think this is ready to merge, but I'd prefer to merge #185 first because rebasing will be easier in one direction than in the other.

richford commented 6 years ago

Rebased onto master after the merge of #185.

richford commented 6 years ago

Rebased again.

jyeatman commented 6 years ago

I just tested this. I don't fully understand the PR. But everything is working great on my laptop. The query string is saving everything properly as well.

richford commented 6 years ago

For clarification, this PR mostly just resolves issue #180 to get rid of the redundant subjects.json file since all of that info is already in subjects.csv. The rest of the changes are just for formatting the data we get from the csv file. If you like the number formatting in the metadata table, then I think we're ready to merge.

arokem commented 6 years ago

This is good. I will follow up removing the parts of the python code that generate the json files, and removing the template json file.