brettviren / moo

ruminants on module oriented programming
GNU General Public License v3.0
4 stars 4 forks source link

Records do not seem to check if sub-record types are correct during assignment #16

Closed alessandrothea closed 3 years ago

alessandrothea commented 3 years ago

Hi again, This is something I stumbled upon a while ago. As the title suggests, it seems that when a record field is assigned, the type of the assigned value is not checked if the field itself is a record.

Example from DUNE-DAQ/dfmodules/schema

hdf5ds.ConfParams has 2 subrecords, filename_parameters of type hdf5ds.HDF5DataStoreFileNameParams and file_layout_parameters of type hdf5ds.HDF5DataStoreFileLayoutParams. In the first part of the example below, they are correctly assigned but swapped in the second part. On a second look at the output, it seems that some guard is in place, as the attempt to assign swapped types is ignored and the default values are shown

import json

# Load configuration types
import moo.otypes
moo.otypes.load_types('dfmodules-HDF5DataStore-schema.jsonnet')

import dunedaq.dfmodules.hdf5datastore as hdf5ds

print("---Correct subrecord assignment---")
print(json.dumps(hdf5ds.ConfParams(
            filename_parameters = hdf5ds.HDF5DataStoreFileNameParams(
                overall_prefix = "aaa",
            ),
            file_layout_parameters = hdf5ds.HDF5DataStoreFileLayoutParams(
            )
        ).pod(), indent=4, sort_keys=True
    ) )

print("---Swapped subrecord assignment---")
print(json.dumps(hdf5ds.ConfParams(
            filename_parameters = hdf5ds.HDF5DataStoreFileLayoutParams(
            ),
            file_layout_parameters = hdf5ds.HDF5DataStoreFileNameParams(
                overall_prefix = "bbb",
            )
        ).pod(), indent=4, sort_keys=True
    ) )

Result


{
    "directory_path": ".",
    "file_layout_parameters": {
        "apa_name_prefix": "APA",
        "detector_name": "TPC",
        "digits_for_apa_number": 3,
        "digits_for_link_number": 2,
        "digits_for_trigger_number": 6,
        "link_name_prefix": "Link",
        "trigger_record_name_prefix": "TriggerRecord"
    },
    "filename_parameters": {
        "digits_for_file_index": 4,
        "digits_for_run_number": 6,
        "file_index_prefix": "",
        "overall_prefix": "aaa",
        "run_number_prefix": "run"
    },
    "max_file_size_bytes": 1048576,
    "mode": "all-per-file",
    "name": "store",
    "type": "HDF5DataStore"
}

{
    "directory_path": ".",
    "file_layout_parameters": {
        "apa_name_prefix": "APA",
        "detector_name": "TPC",
        "digits_for_apa_number": 3,
        "digits_for_link_number": 2,
        "digits_for_trigger_number": 6,
        "link_name_prefix": "Link",
        "trigger_record_name_prefix": "TriggerRecord"
    },
    "filename_parameters": {
        "digits_for_file_index": 4,
        "digits_for_run_number": 6,
        "file_index_prefix": "",
        "overall_prefix": "fake_minidaqapp",
        "run_number_prefix": "run"
    },
    "max_file_size_bytes": 1048576,
    "mode": "all-per-file",
    "name": "store",
    "type": "HDF5DataStore"
}```
brettviren commented 3 years ago

Oops, messed up auto close commit comment.

This fixes.

https://github.com/brettviren/moo/commit/32baaaa7d542d6362e988d8e68c800bd666ea3d5