search

ioos / system-test

IOOS DMAC System Integration Test project
github.com/ioos/system-test/wiki
The Unlicense
7 stars 14 forks source link

date_range/dateRange function names inconsistent in 'Theme_2_Extreme_Events' utilities.py files #141

Closed benjwadams closed 10 years ago

benjwadams commented 10 years ago

In various subdirectories in Theme_2_Extreme_Events, the utilities.py files have inconsistent naming conventions. In some files date_range is used, whereas in others dateRange is used. The prevailing style for function names within the utilities.py files seems to conform to the PEP 8 standard, namely using lowercase and underscore names rather than camelcase function names.

Additionally, in Theme_2_Extreme_Events/Scenario_2A_ModelDataCompare_Waves/Scenario_A_Model_Obs_Compare_Waves.ipynb, the IPython notebook uses attempts to import the date_range function, but there only exists the dateRange function in that folder's utilities.py, and hence importing the function fails. Thus, it is probably desirable to be consistent about how functions are named.

birdage commented 10 years ago

@benjwadams good spot, can you can fix this in a pull request?

benjwadams commented 10 years ago

Do we want to settle on date_range or dateRange for the function name?

birdage commented 10 years ago

@benjwadams what do the other utility files use? as we should be consistent

Bobfrat commented 10 years ago

I vote for date_range but it might be confusing because Pandas has a date_range method as well.

benjwadams commented 10 years ago

@birdage: Some utility files currently use dateRange, others use date_range depending on the subfolder. Try grep -R 'def date' --include=utilities.py in the Theme_2_Extreme_Events folder.

ocefpaf commented 10 years ago

Since we are discussing names why not fes_date_filter, or something similar?

On Tue, Aug 12, 2014 at 4:58 PM, Bob Fratantonio notifications@github.com wrote:

I vote for date_range but it might be confusing because Pandas has a date_range method as well.

— Reply to this email directly or view it on GitHub https://github.com/ioos/system-test/issues/141#issuecomment-51968367.

birdage commented 10 years ago

@benjwadams looks like the vote is in, @ocefpaf i think that is a good idea! looks like its a rename and a pull request :+1:

rsignell-usgs commented 10 years ago

+1 to fes_date_filter