aces / Loris

LORIS is a web-accessible database solution for longitudinal multi-site studies.
GNU General Public License v3.0
141 stars 173 forks source link

Defining default decimal point for non JSON table LINST instruments #9277

Closed regisoc closed 4 weeks ago

regisoc commented 1 month ago

Brief summary of changes

This PR forces the decimal(14,4) (10 digits on the integer part, and 4 digits on the floating point part), instead of decimal(10,0) which is the default for numeric and was truncating floating point digits. It only applies this change for instruments that require their own table (non JSON) and have numeric attribute.

Testing instructions (if applicable)

  1. use tools/generate_tables_sql.php and tools/generate_tables_sql_and_testNames.php with a non JSON LINST instrument with at least a numeric type line.
  2. the resulting SQL should be a valid SQL code that add a decimal(14,4) type for the numeric attribute, instead of a numeric attribute.

Link(s) to related issue(s)

Resolves #9237

ridz1208 commented 1 month ago

@driusan I was thinking we could have a (14,4) default which would be backwards compatible with the old (10,0) default but also allow the LINST standard to define the number of digits and number of decimals in the front end? just likesome fields have min and max?

regisoc commented 1 month ago

@driusan @ridz1208 I like that idea. I can add that as a new configuration if that is ok for both of you.

driusan commented 1 month ago

@regisoc I'm not sure how the option would work but it sounds good to me. (I think the first part of the (x, y) can be derived from the min/max that's already there with something like log10(max(abs(min), abs(max)))) + y.

Should I merge this and you'll do it in another PR or do you want to do it in this one?

regisoc commented 1 month ago

@driusan @ridz1208 I think it can be merged as is. The option can be added later as it will only impact both generate_table... scripts.