> On July 17, 2013, 2:44 p.m., Cameron Goodale wrote: > > http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/files.py, > > lines 286-288 > > <https://reviews.apache.org/r/12452/diff/4/?file=322129#file322129line286> > > > > The DIFF makes it look like we are missing the docstrings for these 3 > > input variables. I see the output docs were updated, perhaps these three > > got dropped in the updating. > > > > Can you check on this and make sure we have docs for lonVarName, > > timeVarName and fileType.
I did forget to copy / paste this into the new docstring after deprecating one of the old functions, good catch! > On July 17, 2013, 2:44 p.m., Cameron Goodale wrote: > > http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/storage/files.py, > > lines 771-778 > > <https://reviews.apache.org/r/12452/diff/4/?file=322129#file322129line771> > > > > Alex since this code is a direct copy of the basemap library I think we > > need to include the Copyright info somewhere per the LICENSE for the > > software (below): > > > > copyright (c) 2011 by Jeffrey Whitaker. > > > > Permission to use, copy, modify, and distribute this software and its > > documentation for any purpose and without fee is hereby granted, > > provided that the above copyright notices appear in all copies and that > > both the copyright notices and this permission notice appear in > > supporting documentation. > > > > Chris will be able to give us some direction on this. I was actually looking for the license header in the source files but couldn't find them originally, but it looks like this can be found in the README file. I'll ask Chris for more clarification on where I should put it, but do you think just placing it in the Purpose section of the docstring would suffice? On July 17, 2013, 2:44 p.m., Alex Goodman wrote: > > Alex, > > > > I looked at Revision 4 and commented on some of the changes, but then > > Revision 5 came along and un-did all the changes from Revision 4 (files.py > > is completely reverted). We need to get a single Patch file that contains > > all the changes to the code, so I hope Revision 6 will be a single patch > > with all the changes you and Kyo intend to make. > > > > If you need help let me know. Thanks for catching this, I wasn't actually exactly sure how to add diffs for two different files to one review board thread so I uploaded them separately, looks like that "reverted" the previous diff. I'll try combining them into one file. - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12452/#review23277 ----------------------------------------------------------- On July 16, 2013, 11:21 p.m., Alex Goodman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12452/ > ----------------------------------------------------------- > > (Updated July 16, 2013, 11:21 p.m.) > > > Review request for Apache Open Climate, Cameron Goodale and Kyo Lee. > > > Repository: climate > > > Description > ------- > > This patch partially addresses CLIMATE-186 in that it can now handle data > with reversed or shifted lat lon grids. We have still not decided on how to > go about how to handle different calendars for daily data, so that patch will > come later. > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/toolkit/do_data_prep.py > 1493845 > > Diff: https://reviews.apache.org/r/12452/diff/ > > > Testing > ------- > > -NCEP Reanalysis Air Temperature Data to check if latitudes and associated > data are reversed properly > -Surface Air Temperature data from CMIP5 model output to test grid shift (eg > transform lon domain from [0, 360) to [-180, 180)) > > > Thanks, > > Alex Goodman > >
