----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12072/#review22365 -----------------------------------------------------------
https://svn.apache.org/repos/asf/incubator/climate/trunk/dataset.py <https://reviews.apache.org/r/12072/#comment45878> The ASF license shouldn't be in the docstring. The docstring should describe what dataset.py is for. The license should be in a comment block. Otherwise Sphinx is going to pick this up and dump it in our docs. https://svn.apache.org/repos/asf/incubator/climate/trunk/dataset.py <https://reviews.apache.org/r/12072/#comment45879> Docstrings start with ''' or """ not single ' or ". Also we don't need the trailing \ https://svn.apache.org/repos/asf/incubator/climate/trunk/dataset.py <https://reviews.apache.org/r/12072/#comment45880> Check http://pythonhosted.org/an_example_pypi_project/sphinx.html#full-code-example out to see an example of good docstring style https://svn.apache.org/repos/asf/incubator/climate/trunk/dataset.py <https://reviews.apache.org/r/12072/#comment45881> Keep space around binary operators len(self.timeValues) - 1 https://svn.apache.org/repos/asf/incubator/climate/trunk/dataset.py <https://reviews.apache.org/r/12072/#comment45884> Just convert the timeResolution to a string once instead of in all these different locations. https://svn.apache.org/repos/asf/incubator/climate/trunk/dataset.py <https://reviews.apache.org/r/12072/#comment45885> Change daysNumber to something more meaningful. 'numberOfDays' or 'daysDifference' or something that gets the "this is a difference between two datetimes in days" idea across. Stop regexing on the TimeDelta objects and just use the built in getters/settings. Checkout datetime.timedelta for more info. https://svn.apache.org/repos/asf/incubator/climate/trunk/dataset.py <https://reviews.apache.org/r/12072/#comment45883> Remove the extra whitespace - Michael Joyce On June 25, 2013, 12:35 a.m., Maziyar Boustani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12072/ > ----------------------------------------------------------- > > (Updated June 25, 2013, 12:35 a.m.) > > > Review request for Apache Open Climate, Cameron Goodale, Chris Mattmann, > Michael Joyce, and Paul Ramirez. > > > Repository: climate > > > Description > ------- > > Based on the new structure of OCW (Open Climate Workbench) [1], the > dataset.py is the critical file that will contain all the functions and > attributes related to any type of dataset (local observations, model file, > RCMED and esg). > This is the first time the dataset.py is developed, therefore it may need to > be modified more as we go ahead. > > [1]: > https://cwiki.apache.org/confluence/display/CLIMATE/Open+Climate+Workbench+API+summary > [STRUCTURE A] > > > Diffs > ----- > > https://svn.apache.org/repos/asf/incubator/climate/trunk/dataset.py > PRE-CREATION > > Diff: https://reviews.apache.org/r/12072/diff/ > > > Testing > ------- > > There are three files attached to this review board that can provide the test > situation for you. > 1- local.py ---> This file is not the final version of local.py. However, it > is a good source to test the dataset.py but a model file[3]. > 2- test.py ---> This file will use local.py[1] and model file[3] to test all > the attributes from dataset.py. > 3- prec.CRCM.ncep.monavg.nc ---> This is the actual model file that can be > used to test the dataset.py (feel free to use your model file and report if > any issue found) > > > File Attachments > ---------------- > > This is not the final version of local.py. However, the dataset.py can be > test by this local.py (code example in "Testing area"). > https://reviews.apache.org/media/uploaded/files/2013/06/25/local.py > This > https://reviews.apache.org/media/uploaded/files/2013/06/25/test.py > dfd > > https://reviews.apache.org/media/uploaded/files/2013/06/25/prec.CRCM.ncep.monavg.nc > > > Thanks, > > Maziyar Boustani > >
