----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11785/#review21687 -----------------------------------------------------------
http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/db.py <https://reviews.apache.org/r/11785/#comment44814> I would imagine that someone calling his function would provide a fully qualified netCD_fileName including the '.nc' extension. http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/db.py <https://reviews.apache.org/r/11785/#comment44815> I like how similar the API is compared with PyNIO http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/db.py <https://reviews.apache.org/r/11785/#comment44816> In this call, you don't append the '.nc' to the incoming filename. I like this convention better. http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/files.py <https://reviews.apache.org/r/11785/#comment44819> Docstring should have all mention of PyNIO dropped from it, and the commented out import of PyNIO should also be removed. http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/files.py <https://reviews.apache.org/r/11785/#comment44820> rm this print statement since no longer used. http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/files.py <https://reviews.apache.org/r/11785/#comment44821> Re-use of the keylist variable makes me think this could be consolidated into a single list comprehension: keys = [key.encode().lower() for key in f.variables.keys()] Could that work? http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/files.py <https://reviews.apache.org/r/11785/#comment44822> Another consolidation opportunity http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/files.py <https://reviews.apache.org/r/11785/#comment44823> Another consolidation here http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/files.py <https://reviews.apache.org/r/11785/#comment44825> I see this pattern a lot in this code. Might consider factoring it out into a simple function, just so the variable parsing is consistent between all functions in this module. http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/files.py <https://reviews.apache.org/r/11785/#comment44826> Another instance of appending '.nc' to the incoming filename http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/files.py <https://reviews.apache.org/r/11785/#comment44827> Good catch on docs, also need to update line 684 with the correct return value. http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/files.py <https://reviews.apache.org/r/11785/#comment44829> Using enumerate() instead of maintaining counters is the preferred method of doing this in python. Instead try this: for index, dataset in enumerate(datasets): # Then just replace all instances of datasetCount with index instead # and you can drop the last line that does the incrementing http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/toolkit/do_data_prep.py <https://reviews.apache.org/r/11785/#comment44831> Yeah buddy! http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/toolkit/process.py <https://reviews.apache.org/r/11785/#comment44832> Great use of modelFile (without the .nc appendage) - Cameron Goodale On June 10, 2013, 9:34 p.m., Kyo Lee wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11785/ > ----------------------------------------------------------- > > (Updated June 10, 2013, 9:34 p.m.) > > > Review request for Apache Open Climate, Cameron Goodale and Alex Goodman. > > > Description > ------- > > PyNio has been replaced by netCDF4. We still have to rewrite plots.py to > replace PyNgl with matplotlib. > https://issues.apache.org/jira/browse/CLIMATE-55 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/db.py > 1491550 > > http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/files.py > 1491550 > > http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/toolkit/do_data_prep.py > 1491550 > > http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/toolkit/process.py > 1491550 > > Diff: https://reviews.apache.org/r/11785/diff/ > > > Testing > ------- > > Multiple testings have been done using cordexAF.cfg > > > Thanks, > > Kyo Lee > >
