search

NASA-PDS / mi-label

Metadata Injector for PDS Labels (MILabel) provides a command-line interface for generating PDS4 Labels using a user provided PDS4 XML template and input (source) data products.
https://nasa-pds.github.io/mi-label/
Other
3 stars 1 forks source link

Product-Tools parsing mode cannot parse units #21

Closed danpolitte closed 3 years ago

danpolitte commented 3 years ago

🐛 Describe the bug

The Product-Tools parser mode is unable to return the units of any quantities in a PDS3 label.

It matters to us at PDS-GEO that Product-Tools mode be able to handle units because it supports some PDS3 features, like {}-delimited arrays, which the default VICAR parsing mode does not. There are many PDS3 labels that have both those features and units, but currently no mode of MILabel that can be used to handle all these features at once.

📜 To Reproduce

Steps to reproduce the behavior:

  1. Create a file problem_illustration.lbl with these contents:

    PDS_VERSION_ID                    = PDS3

PROPERTY_WITH_UNIT = 29.7245241213 <DEGREE>
  2. Create a file template.vm with these contents: 
    The property with a unit:
The unit: $label.getUnits('PROPERTY_WITH_UNIT')
The value: $label.get('PROPERTY_WITH_UNIT')
  3. Switch MILabel to Product-Tools mode via the instructions at "Advanced Usage"/"Changing the Underlying Parser" of this document.
  4. Run MILabel with the above files with pds-generate -t template.vm -p problem_illustration.lbl
  5. Observe the following output in the file problem_illustration.xml: 
    The property with a unit:
The unit: none
The value: 29.7245241213

🕵️ Expected behavior

The desired contents of problem_illustration.xml are:

The property with a unit:
The unit: DEGREE
The value: 29.7245241213

📚 Version of Software Used

Version 1.2.0 (latest stable version at this time)

🩺 Test Data / Additional context

N/A

🏞Screenshots

N/A

🖥 System Info

🦄 Related requirements

⚙️ Engineering Details

jordanpadams commented 3 years ago

@mcayanan pulling out some old brain cells here, but any idea if this was ever supported or if there is a part of the pds3-product-tools API we could use to pull that out?

mcayanan commented 3 years ago

@jordanpadams Man, I had to reach for those brain cells! :D So, I did some quick testing with the tool and digging through code to jog my brain. Turns out that the product-tools parser is capturing the units for numeric types:

https://github.com/NASA-PDS/pds3-product-tools/blob/01b9f73a5fcb199cbb0f036d53a2beed88f80abc/src/main/java/gov/nasa/pds/tools/label/Numeric.java#L115-L122

If I remember how MI Label works, I think it's just a matter of updating the MI label code such that it needs to know that you have a Numeric type that you're trying to translate into the resulting output and then calling this API to extract the units.

Hope this helps!

jordanpadams commented 3 years ago

👍 thanks a ton @mcayanan ! appreciate it.

jpl-jengelke commented 2 years ago

@viviant100 @jordanpadams We probably want to start putting test data into the mi-label repository similar to what we are doing with validate. The path would be something like ${project.basedir}/src/test/resources/github21.

jpl-jengelke commented 2 years ago

@jordanpadams Incidentally, the documentation has changed slightly and is out-of-date.

For instance, 

"${JAVA_HOME}"/bin/java -jar ${MILABEL_JAR} "$@"

... has changed to ... 

${JAVA_CMD} -jar ${GENERATE_JAR} "$@"

This may seem minor, but given a novice or non-developer user, it likely complicates their tasks.

jpl-jengelke commented 2 years ago

This issue passes the tests as described, but it was issued a "conditional pass" based on errors reported while running the tests. However, it is believed the errors are test-related (not a real-world test) and therefore not legitimate bugs. Please see test files in TestRail for more information, and mark this passed if the reports are acceptable.

jordanpadams commented 2 years ago

@viviant100 @jordanpadams We probably want to start putting test data into the mi-label repository similar to what we are doing with validate. The path would be something like ${project.basedir}/src/test/resources/github21.

@jpl-jengelke sounds good to me. btw, I am all for better ideas for where/how we should manage this data. what we do in validate just kind of happened out of the easiest solution in front of us.

jordanpadams commented 2 years ago

This issue passes the tests as described, but it was issued a "conditional pass" based on errors reported while running the tests. However, it is believed the errors are test-related (not a real-world test) and therefore not legitimate bugs. Please see test files in TestRail for more information, and mark this passed if the reports are acceptable.

@jpl-jengelke looks good! you are correct that the error is due to the test file being not valid XML. in the future, we may just want to create valid XML files where possible so the tool can execute successfully for a valid test case. not a huge deal, but something to consider down the road

jordanpadams commented 2 years ago

${JAVA_CMD} -jar ${GENERATE_JAR} "$@"

updated per https://github.com/NASA-PDS/mi-label/commit/c0d52a60e0ac4b904293918f0f79e683534ad035

jpl-jengelke commented 2 years ago

@viviant100 @jordanpadams We probably want to start putting test data into the mi-label repository similar to what we are doing with validate. The path would be something like ${project.basedir}/src/test/resources/github21.

@jpl-jengelke sounds good to me. btw, I am all for better ideas for where/how we should manage this data. what we do in validate just kind of happened out of the easiest solution in front of us.

@jordanpadams I'll work with @viviant100 to create a standard for test data taxonomy and locations.