search

pr-omethe-us / pyked-papers

Various papers published about PyKED
Creative Commons Attribution 4.0 International
0 stars 0 forks source link

IJCK R2C18 & 19 #35

Closed bryanwweber closed 7 years ago

bryanwweber commented 7 years ago

Page 7, line 12, mole-percent, mole-fraction, and mass-fraction include a dash in their name, whereas this is not the case for the example on page 5 line 48 and in the description of the composition element on page 4, line 47. Same issue as #18 holds for page 7, line 17.

bryanwweber commented 7 years ago

Ref: https://github.com/pr-omethe-us/pyked-papers/issues/3#issuecomment-332316621

From Kyle:

I agree that we should be consistent about using hyphen, underscore, or nothing in fields with multiple words. I like hyphens.

bryanwweber commented 7 years ago

I agree with hyphens. However, hyphens aren't valid in Python variable names, and I suspect the problem here is the difference between the Python example and the YAML spec. I haven't checked though...

kyleniemeyer commented 7 years ago

Yeah, but we already replace hyphens for YAML fields that have them, so I think it should be fairly "easy" to do that universally.

bryanwweber commented 7 years ago

Are there schema keywords that use underscores? I thought everything in the schema (i.e., in the YAML) had a hyphen (but I haven't checked so I could definitely be wrong 😄 )

bryanwweber commented 7 years ago

OK I see what happened here. This seems to be a mixing of the old format (when mole-fraction was the key) and the new format (when the composition type is specified by a string). I guess in one of the cases the reviewer points out in the text, we missed updating the format, and in the others, the Python representation of the string has a hyphen that we should probably delete (if its added in PyKED, we should remove it, if its not, then the manuscript is wrong).

bryanwweber commented 7 years ago

Also, didn't we add ppm and ppb as composition types?

EDIT: Oh, no, that was just in the conversion, we converted them to mole fraction

bryanwweber commented 7 years ago

After checking the code (https://github.com/pr-omethe-us/PyKED/blob/master/pyked/chemked.py#L584) it looks like we just grab the value from the property dictionary, so it shouldn't have the hyphens in it, so this is all just a typo in the manuscript. That's good!