-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16757/#review40208
-----------------------------------------------------------



https://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/cli/rcmet_cml.py
<https://reviews.apache.org/r/16757/#comment73142>

    If you need to convert the string to an number use int() or float(). It's 
extremely unlikely that you need to use eval. In the very unlikely case that 
you need to be using eval, then you should be using ast.literal_eval. In this 
case you should be using float(). 



https://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/cli/rcmet_cml.py
<https://reviews.apache.org/r/16757/#comment73143>

    Don't use eval, see above.



https://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/cli/rcmet_cml.py
<https://reviews.apache.org/r/16757/#comment73144>

    The point wasn't to make a function to append to the list. The point was to 
make a function for formatting the dataset object into something 
comprehensible. Your choice ultimately.



https://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/cli/rcmet_cml.py
<https://reviews.apache.org/r/16757/#comment73145>

    Use built in libraries for path stuff



https://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/cli/rcmet_cml.py
<https://reviews.apache.org/r/16757/#comment73147>

    Use os.path for this. Don't make the paths manually.



https://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/cli/rcmet_cml.py
<https://reviews.apache.org/r/16757/#comment73146>

    Is the working directory a path? Are you changing the working directory 
value? Then you're changing a path =D
    
    It seems the entire point of this if-block is so you can incorrectly 
generate a file path on line 373. Use os.path for path related things. You 
don't have to do nit-picky things like adding trailing slashes. Plus, if this 
ends up running on Windows then you don't have to come back through later and 
fix all of these explicit path separators that won't work. You'll thank 
yourself later if you put in the initial effort to do it properly =D


- Michael Joyce


On Jan. 9, 2014, 6:12 p.m., Maziyar Boustani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16757/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2014, 6:12 p.m.)
> 
> 
> Review request for Apache Open Climate.
> 
> 
> Repository: climate
> 
> 
> Description
> -------
> 
> This command line tool is using latest version of OCW (Open Climate 
> Workbench) code to give the capability of evaluating climate model output and 
> observation with using available metrics along with generating plots as 
> result.
> At this time, this tool can accept one model and one observation and to 
> generate contour plots with using BIAS as metric. Supporting multi model and 
> multi observation, more metrics and plots are in coming updates.
> 
> 
> Diffs
> -----
> 
>   
> https://svn.apache.org/repos/asf/incubator/climate/trunk/rcmet/src/main/python/rcmes/cli/rcmet_cml.py
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/16757/diff/
> 
> 
> Testing
> -------
> 
> This tool has been tested with one model [1] and one observation [2].
> Plots were generated successfully.
> 
> [1]: AFRICA_KNMI-RACMO2.2b_CTL_ERAINT_MM_50km_1989-2008_tasmax.nc
> [2]: Dataset_id = 10 and Parameter_id = 39
> 
> 
> Thanks,
> 
> Maziyar Boustani
> 
>

Reply via email to