cdcepi / FluSight-forecast-hub

MIT License
26 stars 54 forks source link

each tasks.json `target_metadata.target_keys.target` should be a string, but is a list of strings #1094

Closed matthewcornell closed 4 days ago

matthewcornell commented 5 days ago

Hi Folks. I believe the target_metadata sections of the file hub-config/tasks.json are invalid. IIUC each target_metadata.target_keys.target should be a string, but in this repo each is a list of strings. For example:

                    "target_metadata": [
                        {
                            "target_id": "flu hosp rate change",
                            "target_name": "week ahead weekly influenza hospitalization rate change",
                            "target_units": "rate per 100,000 population",
                            "target_keys": {
                                "target": [
                                    "wk flu hosp rate change"
                                ]
                            },
                            ...
                        }
                    ]
                },

Instead, I think this example should be:

                    "target_metadata": [
                        {
                            "target_id": "flu hosp rate change",
                            "target_name": "week ahead weekly influenza hospitalization rate change",
                            "target_units": "rate per 100,000 population",
                            "target_keys": {
                                "target": "wk flu hosp rate change"
                            },
                            ...
                        }
                    ]
                },

My references are:

I wonder if this is/should be checked by https://github.com/hubverse-org/hubAdmin/blob/main/R/validate_config.R ?

Thank you.

rborchering commented 4 days ago

Thank you for pointing out this issue! We've been trying to follow the documentation and are wondering about the relationship between this change and using v 3.0.0 vs. v 4.0.0. We haven't had any validation errors with the config file so far, but it seems like this might change if we start using v 4.0.0? Are there any foreseeable issues to removing the brackets if we are planning to use v3.0.0? We've tested validate_config() with the brackets removed and received indication of "valid", but just want to make sure we aren't missing any potential complications. @Annabella-Hines

matthewcornell commented 4 days ago

Hi Rebecca. Tagging @annakrystalli to answer your question.

annakrystalli commented 4 days ago

Hi @rborchering !

Removing the brackets will be fine in both v3.0.0 and v4.0.0. If you don't remove the square brackets though you will start getting validation errors in v4.0.0 because we've made the schema more stringent in order to be able to detect [].

See issue https://github.com/hubverse-org/schemas/issues/97#issuecomment-2416201732 and related PR https://github.com/hubverse-org/schemas/pull/108 for more details.

annakrystalli commented 4 days ago

I should probably add a bit more context. Brackets will not cause validation errors (of the config or of model output submissions) in any R functionality. However, as arrays are encoded differently in e.g. python vs R, not fixing the brackets will cause problems for python functionality like the visualisation software @matthewcornell is working on.

Overall I recommend removing the square brackets as technically they are not allowed. Initially we just weren't sure how to encode the restriction in the schema (and have fallen back on documentation), hence they managed to slip passed validation. We're trying to fix this in v4.0.0

rborchering commented 4 days ago

Thanks Anna and Matt! That's really helpful context. We'll go ahead and remove the brackets. Thanks again for identifying this misalignment!