Skip to content

Removed exceptions from == and != for Time - #2114

Merged
taldcroft merged 7 commits into
astropy:masterfrom
mdmueller:no-time-equality-exception
Mar 1, 2014
Merged

Removed exceptions from == and != for Time#2114
taldcroft merged 7 commits into
astropy:masterfrom
mdmueller:no-time-equality-exception

Conversation

@mdmueller

@mdmueller mdmueller commented Feb 21, 2014

Copy link
Copy Markdown
Contributor

Implemented the suggestion in issue #2082.

taldcroft
taldcroft reviewed Feb 21, 2014
View reviewed changes
Comment thread astropy/time/tests/test_comparisons.py Outdated

@taldcroft taldcroft Feb 21, 2014

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps these two tests could be more explicit in terms of the actual output object type:

assert (t1 == None) is False
assert (t1 != None) is True

@taldcroft

taldcroft commented Feb 21, 2014

Copy link
Copy Markdown
Member

Apart from the one comment this looks great. The Travis failure appears to be a random unrelated failure.

@taldcroft taldcroft mentioned this pull request Feb 27, 2014
Merged
@taldcroft

taldcroft commented Feb 27, 2014

Copy link
Copy Markdown
Member

@AmrAS1 - there is a merge conflict now and your branch needs to be rebased. I just did this and the merge conflict is straightforward to resolve. If you aren't familiar with rebasing I can walk you through the steps, which will look like roughly like:

(astropy)weevil$ git rebase master
First, rewinding head to replay your work on top of it...
Applying: Changed == and != operators in Time so no exceptions are thrown
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging astropy/time/core.py
CONFLICT (content): Merge conflict in astropy/time/core.py
Failed to merge in the changes.
Patch failed at 0001 Changed == and != operators in Time so no exceptions are thrown

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To check out the original branch and stop rebasing run "git rebase --abort".

(astropy)weevil$ open astropy/time/core.py  # edit and remove the HEAD block

(astropy)weevil$ git add astropy/time/core.py

(astropy)weevil$ git rebase --continue
Applying: Changed == and != operators in Time so no exceptions are thrown
Applying: Changed time testing to reflect changes in != and == operators for Time
Applying: Removed superfluous operator strings in calls to _tai_difference
Applying: Made == and != assertions more explicit

@taldcroft

taldcroft commented Mar 1, 2014

Copy link
Copy Markdown
Member

@AmrAS1 - in #1932 _tai_difference got changed to _time_difference. This is causing many Travis failures.

@mdmueller

mdmueller commented Mar 1, 2014

Copy link
Copy Markdown
Contributor Author

Oops, I must have missed that. I'll add another commit.

mhvk
mhvk reviewed Mar 1, 2014
View reviewed changes
Comment thread astropy/time/core.py Outdated

@mhvk mhvk Mar 1, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@AmrAS1 - trivial change, but since time differences are not necessarily in TAI anymore, could you also replace TAI -> time here (and same in __ne__ below)?

@mhvk

mhvk commented Mar 1, 2014

Copy link
Copy Markdown
Contributor

@AmrAS1 - I also had a quick look and apart from the small comment about the docstring, this looks ready to go in.

@taldcroft

taldcroft commented Mar 1, 2014

Copy link
Copy Markdown
Member

The Travis fail is unrelated, merging. Thanks @AmrAS1!

taldcroft added a commit that referenced this pull request Mar 1, 2014
@taldcroft
Merge pull request #2114 from amras1/no-time-equality-exception
be52942 
Removed exceptions from == and != for Time
@taldcroft
taldcroft merged commit be52942 into astropy:master Mar 1, 2014
@mdmueller

mdmueller commented Mar 1, 2014

Copy link
Copy Markdown
Contributor Author

No problem!

@mdmueller
mdmueller deleted the no-time-equality-exception branch March 1, 2014 19:17
@embray embray added this to the v0.4.0 milestone Mar 4, 2014
@embray embray added the time label Mar 4, 2014
@mhvk mhvk mentioned this pull request Jul 11, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

time

Projects

None yet

Milestone

v0.4.0

Development

Successfully merging this pull request may close these issues.

4 participants

@mdmueller @taldcroft @mhvk @embray