search

pr-omethe-us / PyKED

Python interface to the ChemKED database format
https://pr-omethe-us.github.io/PyKED/
BSD 3-Clause "New" or "Revised" License
15 stars 15 forks source link

Which properties should be in common-properties? #67

Open bryanwweber opened 7 years ago

bryanwweber commented 7 years ago

At the moment, the common-properties section allows the following keys:

common-properties:
  type: dict
  schema:
    pressure: *value-unit-optional
    ignition-type: *ignition-type
    composition: *composition
    pressure-rise: *value-unit-optional

How can we handle the common properties without explicitly specifying which ones are allowed? I think that any property should be eligible to be a common-property.

kyleniemeyer commented 6 years ago

I agree that we should allow any property to be in common-properties

kyleniemeyer commented 6 years ago

I'm not sure the easiest way of doing this, but perhaps common-properties: in chemked_schema.yaml can be extended to:

# Start the ChemKED schema
common-properties:
  type: dict
  schema:
    pressure: *value-unit-optional
    ignition-type: *ignition-type
    composition: *composition
    pressure-rise: *value-unit-optional
    compression-time: *value-unit-optional
    ignition-delay: *value-unit-required
    first-stage-ignition-delay: *value-unit-optional
    compressed-pressure: *value-unit-optional
    compressed-temperature: *value-unit-optional
    temperature: *value-unit-required

?

(Though I don't think we should really allow ignition-delay to be common...)

bryanwweber commented 6 years ago

Right, but if we do it that way, we have to add every property from every experiment type that we end up adding. I'm hoping for something a little more sustainable :smile:

(Also, everything in common-properties has to be *value-unit-optional or else it becomes required in the common-properties section, which we don't want)

kyleniemeyer commented 6 years ago

Right... I agree this way is a bit harder to maintain.

I think we have two options:

  1. Don't specify anything for common-properties, just let the YAML parser pull the things that are tagged/referenced into the datapoints. This way, we don't have to edit the allowed fields as we increase the number of potential fields.

  2. If we want to prevent some things from being used here, like ignition-delay, then we probably have to explicitly list the available options in the schema.

I don't have a strong feeling either way, but I'm leaning towards option 2, because explicit is better than implicit :), and I don't think the number of available fields will really increase that much, because other experiment types use much of the same fields we already have (temperature, pressure, composition).

kyleniemeyer commented 6 years ago

Any more thoughts on this?