Switch from getlist to getExpression in config parser

[xylar]

This will support more general python data structures, such as
dictionaries, tuples, etc. It will also support lists of lists,
which will be useful for later work mapping variable names as they
change between MPAS versions.

This changes the syntax supported for lists of strings: they must
now be in single or double quotes.

The function setlist has been removed, since lists and other data
structures can be converted to strings with str(dataStructure).

The default config.analysis and the 4 analysis scripts that used getlist have been updated for the new getExpression syntax.

[xylar]

@pwolfram, this is an enhancement I need in the config parser before getting to work on a solution for #20. It seemed to be best if it got its own PR.

With the change, I am using ast.literal_eval(...) to convert strings with valid python data structures stored in a config file to the associated data structures in the code. This approach is much more general that what is currently supported. For example, it will support lists of lists (the case I will need soon), dictionaries, tuples, etc.

The new approach isn't completely backwards compatible with the old getlist approach. In the old approach, I supported strings without quotes, whereas with literal_eval, python will try to evaluate these unquoted strings as variable names, which will fail. This means we need to update our config files to have quotes around all strings in lists. I have done this in config.analysis. Depending on how different other config files are from the default, it should be pretty easy to patch up other config files based on my example.

I'm not that worried about the fact that config files will have to be changed to support this update. This is because I anticipate other changes before we merge develop to master area also likely to render older config files invalid for the latest version of this repo.

@pwolfram and @milenaveneziani both, let me know if you have concerns about having to update config files to support this new syntax.

[xylar]

Testing

I ran the alpha7 test I have been using for testing (see #47) and verified that the plots are identical to those produced with develop. The updates to the config file are exactly the same as those to config.analysis so I won't bother posting an updated version here.

[xylar]

@pwolfram, once you are happy with this PR, I will squash the commits to keep things cleaner.

[xylar]

@pwolfram and @milenaveneziani , the solution I'm working on in #52 now does not rely on this capability. Nevertheless, as long as you agree it is useful and the fact that config files will need to be updated, I would like to go forward with this PR.

[xylar]

@pwolfram, I just added unit testing for MpasAnalysisConfigParser. It tests getExpression(...) on list, tuple and dict types on a sample config file (as well as more standard things like get, getint, etc.).

This should make the PR easier to review and is more in keeping with the testing approach we want to be moving toward throughout this repo.

[pwolfram]

@xylar, it appears that at the configuration file stage we are now making assumptions related to integers vs floats in things like lists, tuples, dicts etc via config.getExpression. I don't have a problem with this but it could potential cause a strange bug in the future if we aren't careful about this issue. Do we need to have some type of flag to explicitly specify the type? I'm not sure we do but this is one of the downsides to using a language like python (i.e., no explicit typing).

Feel free to ignore Not a Big Deal (NBD) comments for expediency.

[pwolfram]

NBD: probably don't need a comment like this that states essentially the command of the next line of code. This is true of all comments in this method.

[pwolfram]

Thanks for adding this test!

[pwolfram]

@xylar, very nice!!! This removes a ton of code and is a really clean solution.

[pwolfram]

Nice.

[pwolfram]

@xylar, aside from the issue of typing that we should discuss that is probably not a big deal, this looks good to me pending squashing of your commits as previously discussed and verification that this works on alpha/beta following merging with #47, #50, and #51.

[xylar]

it appears that at the configuration file stage we are now making assumptions related to integers vs floats

Yes, that had occurred to me, too.

I don't have a problem with this but it could potential cause a strange bug in the future if we aren't careful about this issue. Do we need to have some type of flag to explicitly specify the type? I'm not sure we do but this is one of the downsides to using a language like python (i.e., no explicit typing).

I'm open to this idea. I would be easy to explicitly type each element (or each value). The default should be leaving everything alone, since we want to support lists of heterogeneous type. I'll push something soon.

[pwolfram]

@xylar, I would prefer we don't change this PR. At this point I think a new PR would be better because I'd like to get all the PRs merged in batch as we discussed earlier, is that ok with you?

[xylar]

Oops, too late. I can revert my last commit. Would you like me to do that?

[pwolfram]

No worries! I can easily roll back my merge and thankfully wasn't fully in the testing loop yet. This is probably cleaner anyway because of the fast turn-arounds.

[pwolfram]

@xylar, I messed this up again similar to what happened with a bad rebase. I accidentally didn't have commit 473db9a included in this PR and consequently it didn't close properly and it left this PR open. When I removed the commit the PR automatically closed. We will have to add it to another PR-- sorry about this huge inconvenience and annoyance.

[xylar]

@pwolfram, these things happen...