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

Setting timezoneOffset gives incorrect time when parsing relative time #175

Closed
osh123 opened this issue Feb 9, 2017 · 11 comments
Closed

Comments

@osh123
Copy link

osh123 commented Feb 9, 2017

process.env.TZ='utc';

var relative = chrono.parse("in two minutes");
relative[0].start.assign('timezoneOffset', 120);

console.log(relative[0].start.date()); //2017-02-09T07:21:17.000Z

var absolute_date = new Date();
absolute_date.setMinutes(absolute_date.getMinutes()+2);

var absolute = chrono.parse(`${absolute_date.getHours()}:${absolute_date.getMinutes()}`);
relative[0].start.assign('timezoneOffset', 120);

console.log(absolute[0].start.date()); //2017-02-09T09:21:00.000Z
@ptxyz
Copy link

ptxyz commented Jun 15, 2017

I also am experiencing this problem. I wonder if it's possible to add a feature to specify the implied or known timezone before the parse occurs?

For instance, if I'm in GMT at 00:00 on January 1 and I parse now it will set the known values as interpreted on my system. If I then apply a timezoneOffset for GMT +0100 and look at the result, it's interpreted as Jan 1 00:00 for GMT +0100. Which means that when I access it locally again, It appears to me as December 31 23:00 at GMT 0000 when it should be January 1 0:00 at GMT 0000 In effect, the timezone offset for these relative phrases simply subtracts the offset from the absolute time.

@wanasit
Copy link
Owner

wanasit commented Jun 17, 2017

...
relative[0].start.assign('timezoneOffset', 120); 
// Should be absolute.start.assign(...)
...

@wanasit
Copy link
Owner

wanasit commented Jun 17, 2017

I think you miss understand the Chrono's ParsedComponents and assign('timezoneOffset', ..).

The component is machine's timezone independent. The component's timezoneOffset is the parsed timezone or the timezone mentioned in the input.

> chrono.parse('Today 12PM JST')[0].start.get('timezoneOffset')
540
> chrono.parse('Today 12PM UTC')[0].start.get('timezoneOffset')
0
> chrono.parse('Today 12PM')[0].start.get('timezoneOffset')
undefined

The date() function create Javascript Date object from the component, which includes adjusting the parsed timezone into your local system (The Date object is also actually machine independent timestamp, but we still need to adjust the mentioned timezone into correct timestamp). If the component doesn't have timezone then, only that point, it assume that the parsed timezone is the same as the current timezone.

> chrono.parse('Today 12PM JST')[0].start.date().toString()
'Sat Jun 17 2017 12:00:00 GMT+0900 (UTC)'
> chrono.parse('Today 12PM UTC')[0].start.date().toString()
'Sat Jun 17 2017 21:00:00 GMT+0900 (UTC)'

// Because I am in Japan (JST)
> chrono.parse('Today 12PM')[0].start.date().toString()
'Sat Jun 17 2017 12:00:00 GMT+0900 (UTC)'

So, if you know that all of your text/input will come from a specific timezone, you could write a refiner that assign the timezone to all the results.

results.forEach(function(result) {
  if (! result.start.isCertain('timezoneOffset')) {
    result.start.assign('timezoneOffset', XXX)
  }
})

But, if you are thinking about using assign('timezoneOffset', ..) to transform the output to into a specific timezone, then you are doing it wrong.

Again, 'timezoneOffset' is all about the timezone mentioned in the text, not output or machine timezone.

  • Do use assign('timezoneOffset', ..) if you know the timezone of the text/input.
  • Do NOT use assign('timezoneOffset', ..) to select output timezone.

@wanasit wanasit closed this as completed Jun 17, 2017
@ptxyz
Copy link

ptxyz commented Jun 17, 2017

Thank you @wanasit for the clear and detailed response. The inner workings of your library are much clearer now after reading this. Respectfully, I still feel that there is a bug in specific cases where the implied values are derived from system time. In these cases, specifying a timezone will result in a time shift. Logically, I understand how this case occurs and it very well may be a case of 'working as designed'

To give you some background, I'm processing text from all over the world. I do know ahead of time which timezone the text will originate from, so I feel comfortable using a refiner as you suggested to set a known value for timezoneOffeset However, if I do set this, then situations like 'now' still present a problem because it appears that the implied time values originate from system time. So it will imply the system hour, minute, second based on the system time.

When I then tell it that I have a known timezone, it's still using the implied values that originated from my system, combined with a known timezone offset, which results in a time shift.

The implication of this is that I would need to conditionally apply the timezoneOffset value only in cases where the implied values are not derived from system time. Since I don't know the nature of the text ahead of time, an ideal situation would allow me to notify the parser to expect inputs to be in specified timezone, and then situations like 'now' or 'in two minutes', where timezone is irrelevant, would simply ignore that value.

I've also provided a runkit notebook to illustrate the problem.
https://runkit.com/pthoresen/5944c4065e466600126d5001

@wanasit
Copy link
Owner

wanasit commented Jun 17, 2017

@pthoresen Thank you for explaining what are you working on.

Here I think, then, you need to adjust your reference time in addition to the timezone.

In example, if I want to interpret "now" correctly, I need to know

  • where it was mentioned, which is timezoneOffset we talked about
  • when it was mentioned. This is the reference time/date.

The reference time should be the time when the text is written. If you don't know that time, you simply cannot know what "now" means.

I am not sure how do you store the time, but if you have Javascript's Date object, the Date object is already timezone-independent timestamp.

var date1 = new Date('Sat Jun 17 2017 12:00:00 GMT+0900 (UTC)')
var date2 = new Date('Sat Jun 17 2017 03:00:00 UTC')
console.log(date1.getTime())
console.log(date2.getTime())

The both outputs of code above should print "1497668400000" regardless of your computer timezone.

If you have the correct reference time and timezone for the input, Chrono should be able to create the correct/precise output.

// Suppose the text is written at this time. At this exact moment.
var ref = new Date('Sat Jun 17 2017 12:00:00 GMT+0900 (UTC)')

// Parse with your custom timzoneOffset=120 chrono
console.log(  custom.parseDate('this Sunday', ref)  ); 
console.log(  custom.parseDate('this Sunday', ref).getTime()  ); 

My first line output print "Sun Jun 11 2017 19:00:00 GMT+0900 (JST)", you will get a different text (but the same meaning) depending on your computer format.

However, the second output will always print "1497175200000".

So, if you correctly pass the "complete" input (text + reference time + timezone), Chrono should work the same way and conclude the same result regardless of local timezone.

@muhajirdev
Copy link

muhajirdev commented Feb 7, 2020

I am assuming the time it's said is right now. And where it's said is in Taipei, (offset 480). It gives me this result

var chronoNode = require("chrono-node")

console.log(new Date())
// Fri Feb 07 2020 08:23:24 GMT+0700 (WIB)

const result = chronoNode.parse('now', new Date())[0].start;
const result2 = chronoNode.parse('in 30 mins', new Date())[0].start;
const result3 = chronoNode.parse('today at 10pm', new Date())[0].start;

result.assign('timezoneOffset', 8 * 60);
result2.assign('timezoneOffset', 8 * 60);
result3.assign('timezoneOffset', 8 * 60);

console.log('result (now)', result.date())
// Fri Feb 07 2020 07:23:24 GMT+0700 (WIB) => This is wrong, should be Fri Feb 07 2020 08:23:24 GMT+0700 (WIB)
console.log('result2 (in 30 mins)', result2.date())
// Fri Feb 07 2020 07:53:24 GMT+0700 (WIB) => This is wrong, should be Fri Feb 07 2020 08:53:24 GMT+0700 (WIB)
console.log('result3 (today at 10pm)', result3.date())
// Fri Feb 07 2020 21:00:00 GMT+0700 (WIB) => This is correct

Did I miss any step?

@tamirvs
Copy link

tamirvs commented Nov 22, 2020

I'm facing the same issue when parsing with reference to user timezone.
Implying the timezone solves cases like 4pm today but fails in cases like in 5 minutes and vice versa.

@wanasit
Copy link
Owner

wanasit commented Nov 23, 2020

Hello. Sorry for very my slow reply.

I am actually not sure what is the problem with @muhajirdev's scenario.
And I am not sure what are you trying to do.

First, you are in GMT+0700 timezone.

console.log(new Date())
// Fri Feb 07 2020 08:23:24 GMT+0700 (WIB)

const result = chronoNode.parse('now', new Date())[0].start;
// result = { hour: 8, minute: 23, second: 24, ..., timezone: 420 (GMT+0700) }

Then, you are assigning 8 * 60, which is GMT+0800 to the result.

result.assign('timezoneOffset', 8 * 60);
// result = { hour: 8, minute: 23, second: 24, ..., timezone: 480 (GMT+0800) }
...

console.log('result (now)', result.date())
// Fri Feb 07 2020 07:23:24 GMT+0700 (WIB) => This is expected 8:23 in GMT+0800 is 7:23 in your timezone

@tamirvs
Copy link

tamirvs commented Nov 23, 2020

@wanasit I'm not sure about @muhajirdev's example but consider this:
We have an input from a user that we know his timezone (e.g. GMT+1). Let's say these 2 inputs:

  1. tomorrow at 5pm
  2. in 10 minutes

Then we assign his timezone:
result.start.assign('timezoneOffset', 60)

Now the first input is parsed correctly, but the second is not.
I think that's because in 10 minutes or now and such, do not depend on timezone, so when you assign a timezone it actually shifts the result.
You can see the same effect when parsing these 2 lines for example:

  1. now
  2. now GMT+2

Which were supposed to have the same result.

I think the fix should be that when the relative parser is used, the timezoneOffset is ignored.

wanasit pushed a commit that referenced this issue Nov 29, 2020

Verified

This commit was signed with the committer’s verified signature. The key has expired.
bamboo Rodrigo Bamboo
@wanasit
Copy link
Owner

wanasit commented Nov 29, 2020

@tamirvs ok. Thank you for the explanation. I think I understand what wrong now.

When we are parsing relative time mention, we should also take into account the reference's timezone (just like we do the reference's hour and minute). This has been implemented in 344026e patch (v2.1.10)

I also added the scenario you mentioned in the en_relative.test.ts

    const refDate = new Date("Sun Nov 29 2020 13:24:13 GMT+0900 (JST)");
    {
        const text = "tomorrow at 5pm";
        const result = chrono.parse(text, refDate)[0] as ParsingResult;
        result.start.imply("timezoneOffset", 60);

        expect(result).toBeDate(new Date("Sun Dec 1 2020 1:00:00 GMT+0900 (JST)"));
        expect(result).toBeDate(new Date("Sun Nov 30 2020 17:00:00 GMT+0100"));
    }

    {
        const text = "in 10 minutes";
        const result = chrono.parse(text, refDate)[0] as ParsingResult;
        result.start.imply("timezoneOffset", 60);

        expect(result).toBeDate(new Date("Sun Nov 29 2020 13:34:13 GMT+0900 (JST)"));
        expect(result).toBeDate(new Date("Sun Nov 29 2020 5:34:13 GMT+0100"));
    }

Note that you suppose to use imply timezone, not assign.

  • The assign function is for hard overriding
  • The imply is for additional guessing, which is what you want.

@tamirvs
Copy link

tamirvs commented Dec 4, 2020

@wanasit It looks like you solved it! thanks!

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

No branches or pull requests

5 participants