Skip to content

Comparisons of Time and TimeDelta#1171

Merged
taldcroft merged 4 commits into
astropy:masterfrom
mhvk:time/comparisons
Jul 6, 2013
Merged

Comparisons of Time and TimeDelta#1171
taldcroft merged 4 commits into
astropy:masterfrom
mhvk:time/comparisons

Conversation

@mhvk

@mhvk mhvk commented Jun 9, 2013

Copy link
Copy Markdown
Contributor

Procrastinating on things I should really be doing, I implemented comparisons of Time and TimeDelta objects, with comparisons only possible if the two are the same type (when it seemed without ambiguity). Setting up with:

from astropy.time import Time, TimeDelta
import numpy as np
t1 = Time(np.arange(49995,50005), format='mjd', scale='utc')
t2 = Time(np.arange(49000,51000,200), format='mjd', scale='utc')

One gets:

In [5]: t1 < t2
Out[5]: array([False, False, False, False, False, False,  True,  True,  True,  True], dtype=bool)

In [6]: t1 == t2
Out[6]: array([False, False, False, False, False,  True, False, False, False, False], dtype=bool)

In [7]: t1[0] > t2[0]
Out[7]: True

In [8]: t1[0] > t2
Out[8]: array([ True,  True,  True,  True,  True, False, False, False, False, False], dtype=bool)

In [9]: t1 > t2[0]
Out[9]: array([ True,  True,  True,  True,  True,  True,  True,  True,  True,  True], dtype=bool)

In [10]: dt = t2-t1
In [11]: t1 > dt
ERROR: OperandTypeError [astropy.time.core]
... (full message omitted) ...

In [12]: dt > TimeDelta(0., format='sec')
Out[12]: array([False, False, False, False, False, False,  True,  True,  True,  True], dtype=bool)

@mhvk

mhvk commented Jun 9, 2013

Copy link
Copy Markdown
Contributor Author

@taldcroft - Should have ping you in the commit and shold also have added hat without this commit, comparisons are already defined, but give output I do not understand the logic of. e.g.,

In [5]: t1 > 0.
Out[5]: True
In [9]: t2 < t1
Out[9]: False
In [15]: t1 > dt
Out[15]: False

@taldcroft

taldcroft commented Jun 10, 2013

Copy link
Copy Markdown
Member

@mhvk - I took a quick look at your compare operations, and it's probably OK but I'll need to think about it a little (and some other obligations are pressing). See also #601.

In quick response to the question about the current behavior of compare, this is just something that is in the object base class. I remember reading what's actually being compared, but I forget, maybe id?

In [1]: class Foo(object):
   ...:     pass
   ...: 

In [2]: a = Foo()

In [3]: b = Foo()

In [4]: a > 0.
Out[4]: True

In [5]: a < b
Out[5]: True

In [6]: b < a
Out[6]: False

@mhvk

mhvk commented Jun 10, 2013

Copy link
Copy Markdown
Contributor Author

@taldcroft - OK, no hurry, and sorry I missed #601. The fact that objects already allow comparisons seems all the more reason to implement something that at least does it more or less right! As you'll have seen, I do the comparison on tai, which is hopefully what people would expect.

@mhvk

mhvk commented Jun 19, 2013

Copy link
Copy Markdown
Contributor Author

@taldcroft - also for these time comparisons, OK to go in?

embray
embray reviewed Jun 27, 2013
View reviewed changes
Comment thread astropy/time/core.py Outdated

@embray embray Jun 27, 2013

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.

This seems a bit redundant. Assuming you didn't want an object with a class that is a subclass of the other object to be comparable, this is equivalent to self.__class__ is other.__class__.

@mhvk

mhvk commented Jul 3, 2013

Copy link
Copy Markdown
Contributor Author

@iguananaut - agreed on the simplification; done.

taldcroft
taldcroft reviewed Jul 5, 2013
View reviewed changes
Comment thread astropy/time/core.py Outdated

@taldcroft taldcroft Jul 5, 2013

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.

Can you just do:

self_tai = self.tai
other_tai = other.tai
return (self_tai.jd1 - other_tai.jd1 + self_tai.jd2 - other_tai.jd2)

I think this should work, but maybe there is something I'm missing. Using those properties instead of the internal _time attributes does the shaped_like_input already, so you could remove the method below and clean up the code a lot. But like I said maybe there was something I didn't think of.

@mhvk

mhvk commented Jul 5, 2013

Copy link
Copy Markdown
Contributor Author

@taldcroft - yes, much nicer. Now done. Also added some tests for all comparisons to ensure I didn't miss one in the cut-and-paste actions.

taldcroft added a commit that referenced this pull request Jul 6, 2013
@taldcroft
Merge pull request #1171 from mhvk/time/comparisons
b4be0da 
Add comparisons of Time and TimeDelta.  Closes #601.
@taldcroft taldcroft merged commit b4be0da into astropy:master Jul 6, 2013
@mhvk mhvk deleted the time/comparisons branch July 6, 2013 19:16
taldcroft added a commit that referenced this pull request Jul 7, 2013
@taldcroft
Update CHANGES.rst
7aaa662 
Update for PR #1171 and #961.
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

table

Projects

None yet

Milestone

v0.3.0

Development

Successfully merging this pull request may close these issues.

3 participants

@mhvk @taldcroft @embray