Unidata / IDV

The Integrated Data Viewer (IDV) from Unidata is a framework for analyzing and displaying geoscience data.
http://www.unidata.ucar.edu/software/idv/
Other
80 stars 37 forks source link

Improved handling of identically-named formula parameters. #35

Open jon4than opened 11 years ago

jon4than commented 11 years ago

Hi everyone,

@mhiley pointed out to me that if a parameter name is shared by more than one formula, the user input written out to operandcache.xml will be associated with every parameter using that name.

This is due to the way the operand cache key is constructed. Currently, it uses the following:

cache[(parameter_name, parameter_type)] = <VALUE>

The attached code simply changes the key to this:

cache[(formula_description, parameter_name, parameter_type)] = <VALUE>

This was a little problematic in that a DataOperand had no way of knowing which formula it is associated with, so I ended up having to modify both DataOperand and DerivedDataChoice. While all of that should be obvious from the diffs, I wanted to explicitly mention it because I feel like it is a little too fragile…though it has worked fine in my testing.

donmurray commented 11 years ago

Is this really a problem? In what scenarios does this cause problems for the user? Just trying to figure out if this is an annoyance or a bug. Mucking with DataOperand and DerivedDataChoice always makes me nervous, so this would need some serious testing in the real world. Also, for things like lat/lon, if they are used in different formulas, I think I'd want the same defaults I had last time, especially if I was running different formulas to compute values over the same region. What happens if a user changes the description of a formula? Do their last preferences get lost (they didn't before)? Inquiring minds want to know.

jon4than commented 11 years ago

Yeah, I agree about changes to DataOperand and DerivedDataChoice being kinda scary, and you raise a very good point about the description possibly changing. I'll have to chew on that a bit.

Anyway, I think this is more of an annoyance…but the current behavior can be confusing, especially when you combine identical parameter names that have different default values. That said, I can see the utility in your lat/lon example.

Maybe if the input field was a drop down containing all previous user input?