Giter VIP home page Giter VIP logo

Comments (15)

leejarvis avatar leejarvis commented on July 22, 2024

I've managed to get 10 minutes to look at this. This problem is actually a little more widespread than first though. First of all, Chronic ignores context in day portions. Now you can probably see the problem with a patch like this. Parsing, for example, 24th of January at 7pm with a past context will return the 23rd of January. Chronic isn't (yet) clever enough to know when it's parsing a time/day portion and nothing else. Chronic loops through Repeaters and re-uses code where it can, meaning it's clever enough to know that a month in the past means 1 month ago, or a year in the past means 1 year ago, but not that a 6pm in the past means yesterday at 6pm (unless it's later than 6pm today).

I'll mark this as a bug and try to find some time in the coming days to take a closer look at it. Thanks for reporting

from chronic.

troy avatar troy commented on July 22, 2024

Thanks @injekt, I appreciate it. That all makes sense.

For any other readers, I think that for user-submitted timestamps, the viable solutions today are supporting a few specific formats, or presenting the parsed version back to the user so they can see the parsed time (ideally in realtime). "Enter what you want" is not feasible, nor is presenting errors on submission (some of the times do parse, they just aren't what the user expected).

from chronic.

leejarvis avatar leejarvis commented on July 22, 2024

OK so I think I need more user input into this issue. I mean, ignoring context for day portions kinda makes sense, but not in all situations. The trouble is, Chronic defaults to a future context when none is supplied, rather than :none, for example. But ignores this for day portions without more textual context. Meaning parsing 2pm at 3pm will return one hour previous, returning tomorrows 2pm would simply make no sense. But I also understand that at 11pm, 7am is more likely to mean tomorrow. There's a couple things we can do about this, either default to a non-specific context, and have the user supply a context when dealing with day portions, or leave it how it is, and assume the user will provide more information (for example 7am tomorrow, 10pm yesterday etc). Or alternatively have a cut off time, but I'm against that because it'll get extremely confusing for users.

from chronic.

troy avatar troy commented on July 22, 2024

When context is not provided, I could see the default going either way. For the default, I think it's more important to document how it behaves than to pick the exact right case - as you said, there isn't one right default for everyone.

However, it should never ignore context when explicitly provided. That was the origin of this issue, and I think a lot more important than getting the default right. If the app developer is willing to spend the effort to decide authoritatively which context is correct for them, it should be honored.

I'd argue that it should raise an exception when it can't honor the context (rather than, say, returning a future time with context of past). Obviously that's not going to happen right now given that it ignores context for certain inputs, but I think that is the correct behavior when one is given. As a library consumer, I don't ever want a future time returned :)

from chronic.

dlupu avatar dlupu commented on July 22, 2024

Hello

I have a similar issue (or ar least it seems).

Chronic.parse('27/7/2011 00:43:57', :context => :future) returns 2011-07-27 00:43:57 +0200
Chronic.parse('27/7/2011 00:43:57', :context => :past) returns 2011-07-26 00:43:57 +0200

I need context => :past in my application. In your opinion, the result of the second example is a bug or it's an expected behavior?

from chronic.

leejarvis avatar leejarvis commented on July 22, 2024

@dlupu Unless I'm missing something, I can't make much sense of your issue.

Chronic.parse('27/7/2011 00:43:57', :context => :past) returns 2011-07-26 00:43:57 +0200

Surely, with a past content, this is exactly what you're expecting?

from chronic.

dlupu avatar dlupu commented on July 22, 2024

Sorry about not being clear @injekt. In fact I'm surprised to see that :context => :past changes the result in this particular case.

The parsed string is not ambiguous at all, so one could expect that both versions return the same result. I don't understand why context information changes the result in this case.

since the string to be parsed is pretty clear

from chronic.

leejarvis avatar leejarvis commented on July 22, 2024

Sorry I'm confused, looking at your examples you say that the first one returns 2011-07-27 00:43:57 +0200 and the last one returns 2011-07-26 00:43:57 +0200, these are both different. Am I missing something?

from chronic.

leejarvis avatar leejarvis commented on July 22, 2024

Oh wait, sorry I read your message wrong. You expect them to be the same? You didn't tell me what Time.now was to go by 👍

from chronic.

dlupu avatar dlupu commented on July 22, 2024

I did not specify any reference time so I guess Chronic uses the current time in that case.

from chronic.

leejarvis avatar leejarvis commented on July 22, 2024

Right, which was what? :P

from chronic.

dlupu avatar dlupu commented on July 22, 2024

About 23 hours ago => Mon, 28 Nov 2011 21:07:15 CET +01:00

from chronic.

wulftone avatar wulftone commented on July 22, 2024

I think he means this:

Time.now #=> 2011-12-29 12:03:14 -0800
Chronic.parse('1-12-1950 00:40', :context => :future) #=> 1950-01-12 00:40:00 -0800
Chronic.parse('1-12-1950 00:40', :context => :past) #=> 1950-01-11 00:40:00 -0800

The context is clearly irrelevant here, since the date is explicit. Context should not affect the date, right?

Or else, context should always affect the date, in which case the :future example should return Jan 13th... but that assumes that :future or :past always implies "a day ahead or a day behind" as opposed to a year or a century, or whatever.

from chronic.

leejarvis avatar leejarvis commented on July 22, 2024

No, you're correct. Context should not affect the date. It doesn't matter if we're in 1959 or 2059, 2011 is still the same year, that with any other attributes you give to Chronic, the date returned is supposed to be contextual, context is useless with an absolute date; or should be, at least.

Trying to find some time over the holidays to hack on Chronic but I'm finding it hard. I will get around to fixing this!

from chronic.

cpmurphy avatar cpmurphy commented on July 22, 2024

@dlupu , I think my patch for issue #176 addresses your problem. With current master, I see

Time.now
=> 2013-04-15 11:56:35 -0500
Chronic.parse('27/7/2011 00:43:57', :context => :past)
=> 2011-07-27 00:43:57 -0500

@wulftone your clarifying examples no longer seem to work -- I just get nil, but

Chronic.parse('1/12/1950 00:40', :context => :past)

now returns 1950-01-12 00:40:00 -0600 as I'd expect (for my timezone).

BTW, @injekt thank you for merging my pull request!

Unfortunately, I think the original bug described by this issue is still present:

Time.now
=> 2013-04-15 12:05:21 -0500
Chronic.parse('6pm', :context => :past)
=> 2013-04-15 18:00:00 -0500

from chronic.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.