quintel / etmoses

Online decision support tool to create local energy situations for neighbourhoods, cities and regions with a time resolution of 15 minutes created and maintained by Quintel – Not maintained
https://moses.energytransitionmodel.com
MIT License
11 stars 3 forks source link

Issues/#1613 2 #1616

Closed grdw closed 6 years ago

antw commented 6 years ago

This looks fine, but in hindsight I'm a bit confused about the storage.volume/storage_volume thing. Since the storage attribute was already whitelisted in ETEngine, is that change actually needed? If so, do you know why?

grdw commented 6 years ago

I see that storage is already whitelisted. But it's volume specifically isn't. When I remove the storage_volume whitelisting the following happens:

screen shot 2018-02-16 at 09 06 32

Maybe if I replaced storage_volume with just storage in the technology importable attributes and let the StorageVolumeAttribute figure out how to fetch the volume from the storage hash? Maybe that will make more sense.

One sec.

antw commented 6 years ago

Just gonna merge this PR for now.

I don't feel that ETMoses should be the one to know that one attribute returns a Hash and the other a Float

FWIW I disagree with this. I don't think ETEngine's public API should change to accommodate a (mostly) dead app like ETMoses. It returns an object/hash for storage because other users of the API might be interested in the other storage keys (decay and cost_per_mwh). Revoking the whitelisting of storage would break those clients.