ageller / Firefly

A WebGL interactive particle viewer
GNU Affero General Public License v3.0
66 stars 13 forks source link

Cannot load settings saved with the GUI into Python #129

Closed zhafen closed 2 years ago

zhafen commented 2 years ago

To reproduce, go to the live example, click Save Settings, and then try loading the data in python via

settings = firefly.data_reader.Settings()
settings.loadFromJSON( '~/Downloads/preset.json' )

A screenshot of the error is attached below. image

agurvich commented 2 years ago

thanks for this issue, i have been silently monitoring this bug while trying to think of a more sustainable way to add hooks to the python frontend without having to manually duplicate it in settings.py (for interacting in the python frontend) and also initViewer.js (for loading up the app) and applyUISelection.js (for making and loading presets).

still trying to think of something elegant for at least a subset of hooks that are straight copying parameters (like zmin) with identical names into the viewerParams object. I'll probably make a file that defines them and their defaults which Python reads in order to create the Settings class' allowable arguments. Haven't settled on an implementation yet in my head and am waiting until I'm struck by inspiration.

agurvich commented 2 years ago

to be clear, the issue here is that I (inadvertently) added a javascript parameter, zmin, to initViewer.js and applyUISelection.js but not settings.py. Error messages like these will crop up every time I do something similar and that is not sustainable IMO. Better to have a single spot where these sorts of parameters are defined so that when I add new hooks they automatically propagate where they are needed (and also we'll be less likely to break presets like we used to do in the past).

ageller commented 2 years ago

We could have a separate file (maybe a yaml file?) that defines all of these things (viewerParams, GUIParams, Settings, etc.). Then that could be read into both Python and Javascript, and if we have the right format, we can know which variable needs to be used where. I haven't thought deeply about this, but it seems that this approach could solve the problem.

agurvich commented 2 years ago

I decided to go with a DefaultSettings.json and DefaultParticleSettings.json to move all settings defaults to a single place.