HDFGroup / hdf5-json

Specification and tools for representing HDF5 in JSON
https://support.hdfgroup.org/documentation/hdf5-json/latest/
Other
73 stars 25 forks source link

h5tojson tool adds fillValue creation property when it is not specified. #20

Closed hyoklee closed 9 years ago

hyoklee commented 9 years ago

I think fillValue should not appear if it is not specified.

The JSON output is form the following file: ftp://ftp.hdfgroup.uiuc.edu/pub/outgoing/NASAHDF/GSSTF_NCEP.3.2008.12.31.he5

         "7bf1e82a-324d-11e5-b18e-005056008d34": {
            "alias": [
                "/HDFEOS INFORMATION/StructMetadata.0"
            ], 
            "creationProperties": {
                "allocTime": "H5D_ALLOC_TIME_LATE", 
                "fillTime": "H5D_FILL_TIME_IFSET", 
                "fillValue": "", 
                "layout": {
                    "class": "H5D_CONTIGUOUS"
                }
gheber commented 9 years ago

Agreed, unless the empty string is the fill value.

ghost commented 9 years ago

My understanding is that zero is the default fill value. If so, there is always going to be a fill value specified. The above case would then be indeed interpreted as having an empty string for the fill value.

hyoklee commented 9 years ago

HDFView says 'Fill value: NONE'.

On the other hand, h5dump -p says:

     FILLVALUE {
        FILL_TIME H5D_FILL_TIME_IFSET
        VALUE  ""
     }

So, I'm confused. Is such dataset considered to have fill value specified?

gheber commented 9 years ago

HDFView says 'Fill value: NONE'. -> means that none was specified, i.e., the default will be used.

jreadey commented 9 years ago

So we are saying if the Fill Value is the default (0 for numeric type, "" for string type), don't specify the fill value.

What about other creation properties? Are there defaults that should be suppressed?

gheber commented 9 years ago

Don't specify the fill value unless the user specified it.

jreadey commented 9 years ago

Checkout this fix: https://github.com/HDFGroup/hdf5-json/pull/21.

jreadey commented 9 years ago

Fix is merged to master.