SolarArbiter / solarforecastarbiter-core

Core data gathering, validation, processing, and reporting package for the Solar Forecast Arbiter
https://solarforecastarbiter-core.readthedocs.io
MIT License
33 stars 21 forks source link

Observation object class duplicate storage & string-type dictionaries #770

Closed MichaelHopwood closed 2 years ago

MichaelHopwood commented 2 years ago

The Observation object (e.g. one below) contains duplicate entries for extra_parameters. Additionally, the dictionary is a string which could be converted to a dictionary by ast.literal_eval.

Observation(name='Natural Energy Laboratory of Hawaii Authority ghi', variable='ghi', interval_value_type='interval_mean', 
interval_length=Timedelta('0 days 00:01:00'), interval_label='ending',
site=Site(name='NREL MIDC Natural Energy Laboratory of Hawaii Authority', latitude=19.728144, longitude=-156.058936, 
elevation=4.0, timezone='Etc/GMT+10', site_id='9fbcf7b6-7e49-11e9-a970-0a580a8003e9', provider='Reference', 
extra_parameters='{"network": "NREL MIDC", "network_api_id": "NELHA", "network_api_abbreviation": "NELHA", 
"observation_interval_length": 1}', climate_zones=('Reference Region 9',)), uncertainty=0.0, observation_id='9fc08b2e-7e49-11e9-9f91-
0a580a8003e9', provider='Reference', extra_parameters='{"network": "NREL MIDC", "network_api_id": "NELHA", 
"network_api_abbreviation": "NELHA", "observation_interval_length": 1, "network_data_label": "Global Horizontal [W/m^2]"}', 
units='W/m^2')

This is likely an easy fix (revolving the output of the request in the APISession class) so it could probably be labeled as a good first issue.

lboeman commented 2 years ago

Hi Michael,

There's actually only one entry for extra_parameters here for the observation, the second set of extra_parameters is a part of the nested Site object. It's a little bit confusing for our reference dataset, because often the Observation's site field has the same extra_parameters as the observation. This is because the api only returns the site_id field where having the redundant information is useful. The APISession tries to be helpful and load all of the metadata of the associated Site object.

The extra parameters field is meant to be an open-format string field. In this case, we've used the extra parameters to store some json for consumption by the solarforecastarbiter.io.reference_observations sub-package. It should not generally be considered parsable as a json object.

Thanks for using the library and opening issues, it is much appreciated.

MichaelHopwood commented 2 years ago

My mistake!