Make run_analysis.py PEP8 compliant

[xylar]

In the process, a variable name was changed because it has the same name as a function being imported later (to prevent potential conflicts).

[xylar]

@pwolfram, I haven't found a better way to do these imports, given that we have to know whether displayToScreen == True before doing all these imports. I am definitely up for suggestions. In general, I really dislike having imports that are not at the top of the file.

[pwolfram]

@xylar, I agree with your concern about not having imports at the top of the file and agree it is aesthetically unpleasing. However, I also agree with you that there isn't a clear way to do this better because all our analysis depends on matplotlib even if tangentially. A bigger concern is probably the commented out lines and lack of organization.

One way to help with these problems that may be a good compromise would be to have a function right after the import section at the top of the file

def load_analysis_modules():
     import matplotlib.pyplot as plt
     # analysis can only be imported after the right MPL renderer is selected
     from mpas_analysis.ocean.ohc_timeseries import ohc_timeseries
     from mpas_analysis.ocean.sst_timeseries import sst_timeseries
     # from mpas_analysis.ocean.nino34_timeseries import nino34_timeseries
     # from mpas_analysis.ocean.mht_timeseries import mht_timeseries
    # from mpas_analysis.ocean.moc_timeseries import moc_timeseries
     from mpas_analysis.ocean.ocean_modelvsobs import ocn_modelvsobs
 
     from mpas_analysis.sea_ice.timeseries import seaice_timeseries
     from mpas_analysis.sea_ice.modelvsobs import seaice_modelvsobs

load_analysis_modules() could be called essentially at line 149 after the selection of the backend. This function would be the first function in the file and may not even have a whitespace after the import section. I'm not sure what makes the most sense but probably would leave one space, not the standard two to designate it is associated with the imports section.

What do you think of this idea? I think it may be the compromise that cleans up this code and helps clarify what is happening by putting each piece closer to its logical and standard position.

[xylar]

@pwolfram, that is a good idea. I will implement this now.

[xylar]

Oops, that doesn't work. The modules are only loaded in the function load_analysis_modules() and then are unloaded as soon as that function finishes. So not a solution, unfortunately.

[pwolfram]

@xylar, sorry about the confusion regarding the idea above that is clearly false. Sorry about the bad idea.

I do have a point of clarification, however: why move all the module imports into the __main__? It seems like it may be cleaner to have them where they were previously because then when we need to add a new analysis the code modification will be much more centralized to allow easier extension by future users not familiar with the whole system. I think in this case it may not be the right design solution to separate the import and run code. Also, I think we should keep __main__ as simple and as short as possible. I think if we have to break PEP8 conventions to ensure greater clarity in the code it is worth it.

[xylar]

@pwolfram, I guess once the imports can't go at the top of the file, it doesn't matter much how you break that convention. I prefer having them together in __main__ so they happen as early as possible and so they are in scope for any functions that would need them. But that's not a practical reason at this point since we don't have any functions other than analysis(...). The other thing is that it's not desirable to have multiple imports of the same thing within and if statement when multiple analyses use the same function (e.g. ocean.modelvsobs) but this doesn't seem to have caused a problem.

In general, what I have read is that every effort should be made in python not to do local imports e.g. within funcitons, so I was doing my best to avoid that approach.

I'm sympathetic to the idea that it should be a simple as possible for future users to add new scripts, so keeping everything in one place as it was before would probably be best. I'll change it back.

[xylar]

Testing:

I ran the same alpha7 test as in #47. Plots are unchanged.

[xylar]

@pwolfram, this should be a quick review. I already checked the results and I verified that the script is PEP8 compliant by the fact that spyder gives me no warnings about this. (I highly recommend using spyder with the PEP8 checks on as a IDE.) So all you really need to do is check the code to make sure it looks reasonable to you. If you like you could make sure a test you typically use runs correctly.

[pwolfram]

LGTM pending testing with combined #47, #50, #51.