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

WIP: datetime filtering fixes #1274

Closed
wants to merge 3 commits into from
Closed

WIP: datetime filtering fixes #1274

wants to merge 3 commits into from

Conversation

sc1f
Copy link
Contributor

@sc1f sc1f commented Jan 4, 2021

This draft PR will contain fixes for Perspective's datetime filters, specifically the issues mentioned in #1242 (and any others that may come up) - I'm using this draft PR as a place for notetaking/progress-checking and commentary.

Tasks:

  • Fix how the datestring is being converted to timestamp
    • == filter compares the Unix timestamps - conversion not working properly?
  • Improve UX for datetime filters
    • Make datestring format more lenient?
    • Add a hint to what formats are supported/make the default format more intuitive?
  • Codify the errors that are happening, so we know exactly what to fix
  • Enable programmatic filters using Date() values, not just strings
    • Implemented already, although needs more tests

@sc1f sc1f added enhancement Feature requests or improvements JS labels Jan 4, 2021
@sc1f
Copy link
Contributor Author

sc1f commented Jan 5, 2021

Issue with == filters on datetimes:

Because Perspective stores datetimes as Unix timestamps (ms since epoch), it performs the == filter by matching timestamp values. Perhaps filter values are being converted to timestamps in local time according to the browser, but Perspective (especially with arrow format data) is storing the timestamp in UTC.

// TZ="US-Eastern" or something like that to assert the timezone
// differences.
// ...I think...
tm utc_tm = *std::gmtime(&tt);
Copy link
Contributor Author

@sc1f sc1f Jan 6, 2021

Choose a reason for hiding this comment

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

@texodus this change fixed date filtering from:

Screen Shot 2021-01-05 at 9 47 23 PM

to:

Screen Shot 2021-01-05 at 9 49 38 PM

I think this issue was around for a while (earliest version I tested was 0.4.8, far before the date parser rewrite). What I think is happening - Date.getTime() always uses UTC, so the timestamps stored by Perspective are in UTC, and then re-localized to local time when they are passed into new Date() when output. However, this function (and its previous implementation, jsdate_to_t_date) pulled the ymd values as local time, which then causes a re-localization on top of an already localized datetime - thus the issue happening in the screenshot above. I would like to verify this with a proper test suite akin to what we have in the Python tests - that will be coming as part of this patch.

What I am unsure about - if datetime values are stored as UTC timestamps and correctly localized, then why does == on a datetime still show an offset:

Screen Shot 2021-01-05 at 10 00 10 PM

This behavior has also been around since 0.4.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests or improvements JS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Datetime and date == filters does not work as expected
1 participant