NIST-ISODB / isotherm-digitizer-panel

Contribution form for NIST adsorption isotherm database implemented using pyviz panel
2 stars 3 forks source link

Feature output #46

Closed dwsideriusNIST closed 4 years ago

dwsideriusNIST commented 4 years ago

Updates / Corrections in this PR:

  1. Corrects 'measurement_type' key to 'category'
  2. Adds composition/concentration descriptors (even if unimportant or null) to single-component isotherm output. One could reasonably argue that the 'concentrationUnits' key could be added in postprocessing. But the composition descriptor is a necessity, as the output JSON contains composition in the 'isotherm_data' block even for single-component isotherms.
  3. Adds a function that performs point corrections on the output JSON, to deal with inconsistencies between the autofill/dropdown menus and key values expected by NIST ISODB. This is a bit 'hacky' at present, as the transforms are very specific. The better solution is for the ISODB API to output the mapping between the various descriptors and the more descriptive menu items and then use pointers to perform the transforms in a generic fashion. (This can be done, I just have to clear time to straighten out the SQL queries and update the php code for the API.) I am definitely open to revisions on this.
dwsideriusNIST commented 4 years ago

My only question is concerning the correct_json function:

Since I understand that there will anyhow be another normalization step to postprocess the raw JSON file, the rationale of which fixes are applied in that step vs already in the raw JSON is not clear to me.

After reviewing all of the conversions in correct_json, we could shift almost everything to postprocessing. All the mappings except for "Select" -> None (null) are available through specific SQL queries to the ISODB tables and those queries can be programmed into new API calls utilized by the JSON postprocessor. So... I'll drastically simplify the pre-output JSON manipulation to just cover the "Select" -> None instances.

ltalirz commented 4 years ago

That's fine with me

dwsideriusNIST commented 4 years ago

I've cut the correct_json function and only included the "Select" -> None sanitization. Looks like we have conflicts with master, though!

ltalirz commented 4 years ago

closing in favor of https://github.com/ltalirz/isotherm-digitizer-panel/pull/53