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

📅 Datepicker: set initial range, use unambiguous date format, use arrow, dropdown for timezone option #96

Merged
merged 11 commits into from
Feb 5, 2024

Conversation

JGreenlee
Copy link
Contributor

set initial date range on datepicker

On the initial load, the user has not selected a date and we fallback to only showing the last week of data. The datepicker should convey this information rather than showing blank initially.

This change sets initial start and end date values to the datepicker. As a result, we will no longer need extra checks for the initial condition when start date or end date have not been set yet. This simplifies the code in a few places.

use unambiguous format on datepicker (D MMM Y)

Consider "1/5/2024" – in the US this seems like January 5, while in most of the world it seems like May 1. Whether we did D/M/Y or M/D/Y, it'd probably cause some confusion since the project is used on an international scale.

So instead of just digits, the new format will be "5 Jan 2024". Unambiguous and still pretty concise.

On the initial load, the user has not selected a date and we fallback to only showing the last week of data. The datepicker should convey this information rather than showing blank initially.

This change sets initial start and end date values to the datepicker. As a result, we will no longer need extra checks for the initial condition when start date or end date have not been set yet. This simplifies the code in a few places.
Consider "1/5/2024" – in the US this seems like January 5, while in most of the world it seems like May 1. Whether we did D/M/Y or M/D/Y, it'd probably cause some confusion since the project is used on an international scale.

So instead of just digits, the new format will be "5 Jan 2024". Unambiguous and still pretty concise.
@JGreenlee
Copy link
Contributor Author

Here is what the datepicker now shows on the initial load:

image

This change adds the `arrow` package as a drop-in replacement for any places where `datetime` was used.
The behavior should not change with this diff - this is only for code cleanliness & consistency with the server

**Note: `arrow` does not have 'min' and 'max' variables like `datetime`. So, these are manually replaced. E.g. min time = 00:00, max time = 11:59, max timestamp = maximum epoch time
@JGreenlee JGreenlee changed the title 📅 Set initial range of datepicker and use unambiguous date format 📅 Datepicker: set initial range, use unambiguous date format, use arrow Jan 22, 2024
@JGreenlee
Copy link
Contributor Author

Switched to using arrow instead of datetime.

We do not import from datetime anymore, although the datetime object is still used internally (by pandas and by arrow itself)

From e-mission#73 (comment)
Implements a dropdown which displays below the datepicker and allows choosing between UTC vs local time as the basis for the date selection queries.
So 'timezone' is passed now as an additional argument to the querying functions.
I noticed this code is repeated and since timezone is passed as a 3rd argument 'tz', it warrants extracting to a separate function
Created a function 'get_ts_range' to use in both places.
As described in the code comment, this code is unused and we are not currently filtering UUIDs by datetime, so updated the code to make this clearer
@JGreenlee JGreenlee changed the title 📅 Datepicker: set initial range, use unambiguous date format, use arrow 📅 Datepicker: set initial range, use unambiguous date format, use arrow, dropdown for timezone option Jan 27, 2024
@JGreenlee
Copy link
Contributor Author

Added a dropdown for UTC time vs. Local timezone (as described in #73 (comment))

UI now looks like this:

image image

This weird formatting was from vscode autoformatting (which I have now disabled for python projects)

I know that Python is a language where whitespace / linebreaks can alter the function of the code so I want to be extra careful here
@JGreenlee JGreenlee marked this pull request as ready for review January 27, 2024 05:06
@JGreenlee
Copy link
Contributor Author

As it turns out, the datepicker does not keep current to "today's" date for the selection.
Today, a user reported that the datepicker only allowed them to retrieve data up to January 15 (despite the date today being January 31).

The reason for this is to do with how Plotly Dash works – the DatePicker component gets generated once when the dashboard is deployed, and does not update on every page load. Our last production deployment was Jan 15, so that is what it is locked to.

Described in https://community.plotly.com/t/refreshing-the-start-date-everyday-for-datepickersingle/22234

According to https://dash.plotly.com/live-updates ("Updates on Page Load"), if we define the page content as a function rather than a variable, it will be generated on each page load / refresh.

@JGreenlee JGreenlee marked this pull request as draft January 31, 2024 19:46
fixes the issue described in e-mission#96 (comment)

To recompute a new date on refresh, "layout" needs to be a function. This required reorganizing the page contents.
Some things can stay static but the things that need to be recomputed are reorganized into "make_" functions.

Inside make_controls() is where we generate the dates, and this gets called on every refresh.

The resulting DOM is the same.
@JGreenlee
Copy link
Contributor Author

Fixed in d9c7df2

To prove that it was actually updating and for the purpose of testing, I injected a simple Span with a fully printed out datetime.

Every time I refresh, the 'seconds' updates, which proves that the controls are being re-computed.
Screen Recording 2024-02-02 at 1 59 11 AM (1)

@JGreenlee JGreenlee marked this pull request as ready for review February 2, 2024 07:12
Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

LGTM other that the single refactor below.

Comment on lines 244 to 246
# trim the time part, leaving only date as YYYY-MM-DD
start_date = start_date[:10] if start_date else None
end_date = end_date[:10] if end_date else None
Copy link
Contributor

Choose a reason for hiding this comment

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

this code is repeated multiple times and should be pulled out into a separate function.
This is not just for lowering the lines of code (from 2 to 1) but consolidating the calls so that if we ever change the display/parse method, we can do so in one place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be much better! I didn't see any existing files where a function like that would make sense to go in.
So to refactor, I think I will create a new file utils/datetime_utils.py with this new function, and maybe also the get_ts_range function (which I had created in db_utils)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

refactored

This code was repeated many times and should be extracted to a separate function. Did it in such a way that it accepts any number of inputs, thanks to Python's *args syntax

Put this in a new file since none of the other 'utils' files had anything to do with datetime
This function is used by db_utils but might be better categorized in datetime_utils.
Previously called`get_ts_range`, its new name is `iso_range_to_ts_range`, since I thought that was more descriptive.

-- Also, I noticed `arrow` was imported in db_utils twice, so i fixed that while I was here
@shankari shankari merged commit 5a96e9e into e-mission:master Feb 5, 2024
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

Successfully merging this pull request may close these issues.

2 participants