search

sot / chandra_time

Convert between various time formats relevant to Chandra.
https://sot.github.io/Chandra.Time
BSD 3-Clause "New" or "Revised" License
4 stars 1 forks source link

Iss7 greta float bug #8

Closed jeanconn closed 11 years ago

jeanconn commented 11 years ago

Relax regular expressions that "match" greta to work with ints and floats with fewer than 6 printing decimals (after the str -> float conversions within Chandra.Time).

taldcroft commented 11 years ago

@jeanconn - We need to coordinate better. I did the same thing and just now discovered your work. Sigh. Compare with fix-greta branch. Gotta run to the airport now.

taldcroft commented 11 years ago

Also, start using "hub" so that your branches don't need the issue number encoded in front. Make a branch, then attach to an issue as needed.

jeanconn commented 11 years ago

I figured you'd get my pull request before you'd have a chance to do anything about this on a Sunday when you are also on travel... Oh well. At least independent work is a good sanity check on the possible ways to fix this. What are your thoughts on allowing a 7 digit int ?

taldcroft commented 11 years ago

What are your thoughts on allowing a 7 digit int ?

Does this not already work? One of the first steps in convert is to try casting the value as float and keep that if it succeeds. So the only thing that gets seen in the regex matching is always a float. (I think. Now I'm in a supershuttle to the airport working on tethered wifi and it seems just ridiculous to bring up NX and try it out).

jeanconn commented 11 years ago

Yep, that's probably wrong (though the string formatting works, so harder to recognize a flub there). And OK. Though was that your response to my "allowing a 7 digit int" question? Because that was more a "should there be an expectation that this will work" question:

self.assertEqual(DateTime('2013055', format='greta').date, '2013:055:00:00:00.000')

jeanconn commented 11 years ago

Ah, OK. That makes sense.

taldcroft commented 11 years ago

I don't know if there is a spec for GRETA datetime format, but I doubt the integer representation is formally allowed. Esp. since in matlab all values are floats. The only question is if we were to take care to disallow a pure int, and that seems unnecessary.

taldcroft commented 11 years ago

On the communication front, I do now see your original PR in my mail, but I would swear it wasn't there before.

jeanconn commented 11 years ago

And I'll close this PR with:

Superceded by e1f9522121