Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renaming module part 2. #13

Merged
merged 1 commit into from
Mar 19, 2015
Merged

Renaming module part 2. #13

merged 1 commit into from
Mar 19, 2015

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Mar 17, 2015

Should Fix #12

@rhattersley
Copy link
Member

Looks like this needs rebasing.

@ocefpaf
Copy link
Member Author

ocefpaf commented Mar 17, 2015

@rhattersley not anymore 😉

@rhattersley
Copy link
Member

I did a quick grep for "iris" and "unit>" and it threw up a couple of things:

  1. IRIS_EPOCH
  2. Lots of import unit and subsequent unit.<method> calls in the docstrings.

@ocefpaf
Copy link
Member Author

ocefpaf commented Mar 17, 2015

Is IRIS_EPOCH -> EPOCH OK?

@rhattersley
Copy link
Member

Is IRIS_EPOCH -> EPOCH OK?

Yeah, seems OK.

@@ -17,4 +17,7 @@

__version__ = '0.0.1'

from .unit import *
from .cf_units import *
from .cf_units import (CALENDAR_STANDARD, CALENDARS, UT_NAMES, UT_DEFINITION,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhattersley should I those to __all__ in cf_units.py or is this OK?

@ocefpaf
Copy link
Member Author

ocefpaf commented Mar 17, 2015

@rhattersley this time I tried to be very careful with the renaming and I believe we should be OK now. I also ran nosetests --verbose --with-doctest cf_units to check the doctests and all but one passed:

File "cf_units.py", line 1961, in cf_units.cf_units.Unit.date2num
Failed example:
    u.date2num(datetime.datetime(1970, 1, 1, 5))
Expected:
    5.00000000372529
Got:
    5.0000000037252903

But I don't think that this precision is an issue.

@ocefpaf
Copy link
Member Author

ocefpaf commented Mar 19, 2015

@rhattersley I want to start adding the tests (#2), but I want to avoid adding them and then re-naming everything. So I am waiting for this one to start the test migration.

Do you think this PR is OK to go?

@rhattersley
Copy link
Member

Do you think this PR is OK to go?

It's close enough. 😁

rhattersley added a commit that referenced this pull request Mar 19, 2015
@rhattersley rhattersley merged commit 38fff59 into SciTools:master Mar 19, 2015
@ocefpaf ocefpaf deleted the rename_module branch March 19, 2015 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename the module?
2 participants