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

feat: Javascript DatePicker Implementation #667

Merged
merged 49 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
6d8433e
ui.date_picker JS side implementation
dgodinez-dh Jul 24, 2024
72fdecf
install internationalized date
dgodinez-dh Jul 25, 2024
70a3886
deserialize value props
dgodinez-dh Jul 25, 2024
f5b48ef
isUnavailable callback
dgodinez-dh Jul 25, 2024
73131cc
review suggestions and unit test
dgodinez-dh Jul 25, 2024
8f86f73
fix nullable types
dgodinez-dh Jul 26, 2024
1bc8a2b
make component annotation
dgodinez-dh Jul 29, 2024
5f7663d
dates to iso strings
dgodinez-dh Jul 29, 2024
fbdf5c8
add is_instant_string
dgodinez-dh Jul 30, 2024
924c795
use time zone setting
dgodinez-dh Jul 30, 2024
863e20d
update unit tests
dgodinez-dh Jul 30, 2024
cbf58d2
comment out unavailable values
dgodinez-dh Jul 30, 2024
79b1ee4
doc updates
dgodinez-dh Jul 30, 2024
438e1ba
merge latest
dgodinez-dh Jul 30, 2024
c0ca90e
fix granularity
dgodinez-dh Jul 30, 2024
0d6f1a9
date picker doc
dgodinez-dh Jul 30, 2024
e3bf54b
remove commented code
dgodinez-dh Jul 30, 2024
8e9be6c
fix python test
dgodinez-dh Jul 30, 2024
25a92ea
update test images
dgodinez-dh Jul 31, 2024
8461dce
update docs
dgodinez-dh Aug 1, 2024
4480e6b
merge latest
dgodinez-dh Aug 5, 2024
150db78
Update plugins/ui/docs/components/date_picker.md
dgodinez-dh Aug 5, 2024
9e87c6a
Update plugins/ui/src/js/src/elements/hooks/useDatepickerProps.ts
dgodinez-dh Aug 5, 2024
c489445
Merge branch 'dag_DatePickerUi' of https://github.com/dgodinez-dh/dee…
dgodinez-dh Aug 5, 2024
fe4f2f0
fix onChange called on mount
dgodinez-dh Aug 5, 2024
e6650b3
fix error in log while typing
dgodinez-dh Aug 5, 2024
82c1425
add issue number for unavailable dates
dgodinez-dh Aug 5, 2024
2733b1e
add time table example
dgodinez-dh Aug 5, 2024
53a50cd
Update plugins/ui/docs/components/date_picker.md
dgodinez-dh Aug 6, 2024
f9b4978
Update plugins/ui/docs/components/date_picker.md
dgodinez-dh Aug 6, 2024
44b6ce5
Update plugins/ui/docs/components/date_picker.md
dgodinez-dh Aug 6, 2024
e5d65e1
Update plugins/ui/docs/components/date_picker.md
dgodinez-dh Aug 6, 2024
b1c1fba
Update plugins/ui/docs/components/date_picker.md
dgodinez-dh Aug 6, 2024
4784e12
fix wrapped date callable
dgodinez-dh Aug 6, 2024
afdf9a4
update docs
dgodinez-dh Aug 7, 2024
d97643e
merge latest
dgodinez-dh Aug 7, 2024
84ea457
more doc updates
dgodinez-dh Aug 7, 2024
3b1bc5e
string parsing defaults to zdt
dgodinez-dh Aug 7, 2024
a68597d
fix unit test
dgodinez-dh Aug 8, 2024
ea349ce
more doc updates
dgodinez-dh Aug 8, 2024
dac5a6f
more examples
dgodinez-dh Aug 8, 2024
c2f00c5
doc example fixes
dgodinez-dh Aug 8, 2024
4fd4365
merge latest
dgodinez-dh Aug 19, 2024
910d544
server side placeholder default
dgodinez-dh Aug 19, 2024
5462e82
merge latest
dgodinez-dh Aug 19, 2024
7a86f21
handle time zone for uncontrolled date pickers
dgodinez-dh Aug 19, 2024
183b620
update images
dgodinez-dh Aug 19, 2024
9c6a007
Revert "update images"
dgodinez-dh Aug 19, 2024
c399f1c
update images
dgodinez-dh Aug 19, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 16 additions & 17 deletions plugins/ui/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -1020,11 +1020,12 @@ A tabs component can be used to organize content in a collection of tabs, allowi

###### Parameters

| Parameter | Type | Description |
| ----------------------- | ------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `*children` | `Tab \| TabList \| TabPanels` | The tab panels to render within the tabs component. |
| `on_change` | `Callable[[Key], None] \| None` | Alias of `on_selection_change`. Handler that is called when the tab selection changes. |
| `**props` | `Any` | Any other [Tabs](https://react-spectrum.adobe.com/react-spectrum/Tabs.html#tabs-props) prop
| Parameter | Type | Description |
| ----------- | ------------------------------- | ------------------------------------------------------------------------------------------- |
| `*children` | `Tab \| TabList \| TabPanels` | The tab panels to render within the tabs component. |
| `on_change` | `Callable[[Key], None] \| None` | Alias of `on_selection_change`. Handler that is called when the tab selection changes. |
| `**props` | `Any` | Any other [Tabs](https://react-spectrum.adobe.com/react-spectrum/Tabs.html#tabs-props) prop |

|

|
Expand Down Expand Up @@ -1340,7 +1341,6 @@ ui.date_picker(
default_value: Date | None = None,
min_value: Date | None = None,
max_value: Date | None = None,
unavailable_values: Sequence[Date] | None = None,
granularity: Granularity | None = None,
on_change: Callable[[Date], None] | None = None,
**props: Any
Expand All @@ -1349,17 +1349,16 @@ ui.date_picker(

###### Parameters

| Parameter | Type | Description |
| -------------------- | -------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `placeholder_value` | `Date \| None` | A placeholder date that influences the format of the placeholder shown when no value is selected. Defaults to today at midnight in the user's timezone. |
| `value` | `Date \| None` | The current value (controlled). |
| `default_value` | `Date \| None` | The default value (uncontrolled). |
| `min_value` | `Date \| None` | The minimum allowed date that a user may select. |
| `max_value` | `Date \| None` | The maximum allowed date that a user may select. |
| `unavailable_values` | `Sequence[Date] \| None` | A list of dates that cannot be selected. |
| `granularity` | `Granularity \| None` | Determines the smallest unit that is displayed in the date picker. By default, this is `"DAY"` for `LocalDate`, and `"SECOND"` otherwise. |
| `on_change` | `Callable[[Date], None] \| None` | Handler that is called when the value changes. The exact `Date` type will be the same as the type passed to `value`, `default_value` or `placeholder_value`, in that order of precedence. |
| `**props` | `Any` | Any other [DatePicker](https://react-spectrum.adobe.com/react-spectrum/DatePicker.html) prop, with the exception of `isDateUnavailable`, `validate`, and `errorMessage` (as a callback) |
| Parameter | Type | Description |
| ------------------- | -------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --- |
| `placeholder_value` | `Date \| None` | A placeholder date that influences the format of the placeholder shown when no value is selected. Defaults to today at midnight in the user's timezone. |
| `value` | `Date \| None` | The current value (controlled). |
| `default_value` | `Date \| None` | The default value (uncontrolled). |
| `min_value` | `Date \| None` | The minimum allowed date that a user may select. |
| `max_value` | `Date \| None` | The maximum allowed date that a user may select. | |
| `granularity` | `Granularity \| None` | Determines the smallest unit that is displayed in the date picker. By default, this is `"DAY"` for `LocalDate`, and `"SECOND"` otherwise. |
| `on_change` | `Callable[[Date], None] \| None` | Handler that is called when the value changes. The exact `Date` type will be the same as the type passed to `value`, `default_value` or `placeholder_value`, in that order of precedence. |
| `**props` | `Any` | Any other [DatePicker](https://react-spectrum.adobe.com/react-spectrum/DatePicker.html) prop, with the exception of `isDateUnavailable`, `validate`, and `errorMessage` (as a callback) |

```py

Expand Down
131 changes: 131 additions & 0 deletions plugins/ui/docs/components/date_picker.md
Copy link
Member

Choose a reason for hiding this comment

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

This interface feels like it has an incomplete integration with the existing time libraries. In particular, business calendars are not supported. Business calendars could be used to limit selected dates or times, they can be used to get a appropriate time zone, etc. Business calendars could be added as a future enhancement, but I wanted to note it here.
https://deephaven.io/core/pydoc/code/deephaven.calendar.html
https://github.com/deephaven/deephaven-core/blob/main/engine/time/src/main/java/io/deephaven/time/calendar/BusinessCalendar.java

Copy link
Member

Choose a reason for hiding this comment

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

We're looking at adding the unavailable dates afterwards: #698
Didn't want that to hold up the the rest of the date picker.

Copy link
Member

Choose a reason for hiding this comment

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

date_picker interprets date-time strings in a way that is inconsistent with all other DH time libraries. For example, it interprets any UTC string only as an Instant. All other time libraries accept the same date-time format for either an Instant or a ZonedDateTime. The inconsistency introduced by date_picker is not good and will be confusing to explain and support. I think the inconsistency can be resolved cleanly by introducing a date_type or string_format parameter that can take values of LocalDate, Instant, and ZonedDateTime. This could then be used to control how the string is parsed.

Note: I have a PR to introduce a new format string that is ZDT specific, but there has been enough arguing that I have not pushed it forward. deephaven/deephaven-core#5069

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per discussion, we are going to default datetime strings to ZDT. Then update the documentation to clarify which value input types will produce which output types.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think the widget has a way to select a time zone via clicking. I can imagine that this would be useful. The adobe docs don't seem to mention it, so it might not be available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the DatePicker will display a timezone but does not allow to change it.
For the current implementation, the DatePicker will display the TZ from the user settings and update if the user changes the TZ in the settings. This is for an Instant. If the user uses a ZDT, then the DatePicker is locked to the TZ provided by the ZDT.

Copy link
Member

Choose a reason for hiding this comment

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

The key java time type that is not included in this widget is LocalTime (https://deephaven.io/core/docs/conceptual/time-in-deephaven/#natively-supported-date-time-types). Should it be supported in this widget or in a different one?

Copy link
Member

Choose a reason for hiding this comment

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

There is a question about what return values should result from string inputs. There was a very long discussion around this topic when we created the query language time and calendar library. Ultimately, we decided to make everything as java time centric as possible. As a result, almost all return types are Java time types. There are a few cases where local-date-strings can be provided as inputs and they will return local-date-strings as outputs, but I think these were only included to make it easier to work with tables using strings as date columns -- which is very common.

As a result, I think it is least confusing to always return a java time value. If there is a compelling case to do something different, we should discuss that specific case.

Having said that, it doesn't look like we have exposed the string formatting methods (e.g. https://github.com/deephaven/deephaven-core/blob/main/engine/time/src/main/java/io/deephaven/time/DateTimeUtils.java#L4273) in python. (https://deephaven.io/core/pydoc/code/deephaven.time.html). Do we need to do this?

Notes:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per discussion, we are going to stick with Java date types. User can convert to python / strings as needed.

Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
# Date Picker

Date Pickers allow users to select a Date and Time from a pop up Calendar.

## Example

```python
from deephaven import ui

dp = ui.date_picker(
label="Date Picker",
value="2024-01-02T10:30:00 UTC",
on_change=print,
)
mofojed marked this conversation as resolved.
Show resolved Hide resolved
```

## Date types

There are three types that can be passed in to the props that control the date format:
Copy link
Member

Choose a reason for hiding this comment

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

Only supporting these 3 inputs types is probably excessively restrictive. Our to_j_* methods can handle far more types. For example, to_j_instant can take Instant, int, str, datetime.datetime, numpy.datetime64, and pandas.Timestamp inputs. to_local_date can take about the same inputs, so supporting all of these types would probably need the date_type parameter suggested above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per discussion, we do support those other input types. Will need to update docs to clarify inputs and outputs.


1. `LocalDate`: A LocalDate is a date without a time zone in the ISO-8601 system, such as "2007-12-03" or "2057-01-28".
This will create a date picker with a granularity of days. No time zone will be displayed.
2. `Instant`: An Instant represents an unambiguous specific point on the timeline, such as "2021-04-12T14:13:07 UTC".
This will create a date picker with a granularity of seconds in UTC. The date picker will render the time zone from the user settings.
3. `ZonedDateTime`: A ZonedDateTime represents an unambiguous specific point on the timeline with an associated time zone, such as "2021-04-12T14:13:07 America/New_York".
This will create a date picker with a granularity of seconds in the specified time zone. The date picker will render the specified time zone.

The format of the date picker and the type of the value passed to the `on_change` handler
is determined by the type of the following props in order of precedence:

1. `value`
2. `default_value`
3. `placeholder_value`
Comment on lines +41 to +43
Copy link
Member

Choose a reason for hiding this comment

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

Do they get an error if value and default_value are inconsistent types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried this in various permutations.
There is no error. Instead one of the types "wins" in order of value, default_value, placeholder_value
The docs discuss this as on_change being determined in that precedence.

Comment on lines +41 to +43
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to have more specifics on these parameters here. They are included in DESIGN.md. For example, knowing the default values is critical to know.

Copy link
Member

Choose a reason for hiding this comment

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

In DESIGN.md, the default is described as Defaults to today at midnight in the user's timezone. I assume that doc should also indicate that it is a ZonedDateTime. There is an enormous amount of ambiguity in what the "user's timezone" is. Possibilities are:

  1. The local machine time zone.
  2. The Java default time zone.
  3. The time zone of the default calendar.
  4. The time zone configured for rendering in the UI.

There needs to be a discussion about which is most appropriate and for what reason. The docs then need to be very clear on the default.

Notes:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comes from the Spectrum docs:

A placeholder date that influences the format of the placeholder shown when no value is selected. Defaults to today's date at midnight.

So it must be defaulting to the local machine time zone.


If none of these are provided, the `on_change` handler will be passed an `Instant`.

## UI recommendations

Recommendations for creating clear and effective date pickers:

1. Using the Date Type `Instant` will be most compatible with other Deephaven components.
Copy link
Member

Choose a reason for hiding this comment

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

This is a weird recommendation because doing everything in UTC is the least useful presentation for a user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in an update.


## Events

Date Pickers accept a value to display and can trigger actions based on events such as setting state when changed. See the [API Reference](#api-reference) for a full list of available events.

```python
from deephaven import ui
from deephaven.time import to_j_local_date, dh_today, to_j_instant, to_j_zdt

zoned_date_time = to_j_zdt("1995-03-22T11:11:11.23142 UTC")
instant = to_j_instant("2022-01-01T00:00:00 ET")
Copy link
Member

Choose a reason for hiding this comment

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

Not a great example because the ZDT is UTC, which looks like the same behavior as Instant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in an update.

local_date = to_j_local_date(dh_today())


@ui.component
def date_picker_test(value):
date, set_date = ui.use_state(value)
return [ui.date_picker(on_change=set_date, value=date), ui.text(str(date))]


zoned_date_picker = date_picker_test(zoned_date_time)
instant_date_picker = date_picker_test(instant)
local_date_picker = date_picker_test(local_date)
```

## Variants

Date Pickers can have different variants to indicate their purpose.

```python
from deephaven import ui


@ui.component
def date_picker_variants():
return [
ui.date_picker(description="description"),
ui.date_picker(error_message="error", validation_state="valid"),
ui.date_picker(error_message="error", validation_state="invalid"),
ui.date_picker(min_value="2024-01-01", max_value="2024-01-5"),
Copy link
Member

Choose a reason for hiding this comment

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

2024-01-5 looks like it should be an invalid ISO date.
https://en.wikipedia.org/wiki/ISO_8601#Calendar_dates

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in an update.

ui.date_picker(value="2024-07-27T16:10:10 America/New_York", hour_cycle=24),
ui.date_picker(granularity="YEAR"),
ui.date_picker(granularity="MONTH"),
ui.date_picker(granularity="DAY"),
ui.date_picker(granularity="HOUR"),
ui.date_picker(granularity="MINUTE"),
ui.date_picker(granularity="SECOND"),
]


date_picker_variants_example = date_picker_variants()
```
mofojed marked this conversation as resolved.
Show resolved Hide resolved

## Time table filtering

Date Pickers can be used to filter tables with time columns.

```python
from deephaven.time import dh_now
from deephaven import time_table, ui


@ui.component
def date_table_filter(table, start_date, end_date, time_col="Timestamp"):
after_date, set_after_date = ui.use_state(start_date)
before_date, set_before_date = ui.use_state(end_date)
return [
ui.date_picker(
label="Start Date", value=after_date, on_change=set_after_date
),
ui.date_picker(
label="End Date", value=before_date, on_change=set_before_date
),
table.where(f"{time_col} >= after_date && {time_col} < before_date"),
]


SECONDS_IN_DAY = 86400
today = dh_now()
_table = time_table("PT1s").update_view(
["Timestamp=today.plusSeconds(SECONDS_IN_DAY*i)", "Row=i"]
)
date_filter = date_table_filter(_table, today, today.plusSeconds(SECONDS_IN_DAY * 10))
```

## API Reference

```{eval-rst}
.. dhautofunction:: deephaven.ui.date_picker
```
52 changes: 49 additions & 3 deletions plugins/ui/src/deephaven/ui/_internal/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import sys
from functools import partial
from deephaven.time import to_j_instant, to_j_zdt, to_j_local_date
from deephaven.dtypes import ZonedDateTime, Instant

from ..types import Date, JavaDate

Expand All @@ -18,6 +19,8 @@
"java.time.LocalDate": to_j_local_date,
}

_LOCAL_DATE = "java.time.LocalDate"


def get_component_name(component: Any) -> str:
"""
Expand Down Expand Up @@ -211,6 +214,19 @@ def create_props(args: dict[str, Any]) -> tuple[tuple[Any, ...], dict[str, Any]]
return children, props


def _is_instant_string(value: str) -> bool:
"""
Check if a string should parse as an Instant.

Args:
value: The string to check.

Returns:
Whether the string should parse as an Instant.
"""
return value.endswith("Z") or value.endswith("UTC")


def _convert_to_java_date(
date: Date,
) -> JavaDate:
Expand All @@ -225,6 +241,15 @@ def _convert_to_java_date(
Returns:
The Java date type.
"""
# For strings, parseInstant and parseZonedDateTime both succeed for the same strings
# To differentiate, we check if the string looks like an Instant otherwise we default to ZonedDateTime
if isinstance(date, str) and not _is_instant_string(date):
try:
return to_j_zdt(date) # type: ignore
except Exception:
# ignore, try next
pass

try:
return to_j_instant(date) # type: ignore
except Exception:
Expand Down Expand Up @@ -288,7 +313,16 @@ def _wrap_date_callable(
Returns:
The wrapped callable.
"""
return lambda date: wrap_callable(date_callable)(converter(date))
# When the user is typing a date, they may enter a value that does not parse
# This will skip those errors rather than printing them to the screen
def no_error_date_callable(date: Date) -> None:
wrapped_date_callable = wrap_callable(date_callable)
try:
wrapped_date_callable(converter(date))
except Exception:
pass

return no_error_date_callable


def _get_first_set_key(props: dict[str, Any], sequence: Sequence[str]) -> str | None:
Expand Down Expand Up @@ -341,7 +375,7 @@ def _prioritized_callable_converter(
def convert_list_prop(
key: str,
value: list[Date] | None,
) -> list[JavaDate] | None:
) -> list[str] | None:
"""
Convert a list of Dates to Java date types.

Expand All @@ -357,14 +391,15 @@ def convert_list_prop(

if not isinstance(value, list):
raise TypeError(f"{key} must be a list of Dates")
return [_convert_to_java_date(date) for date in value]
return [str(_convert_to_java_date(date)) for date in value]


def convert_date_props(
props: dict[str, Any],
simple_date_props: set[str],
callable_date_props: set[str],
priority: Sequence[str],
granularity_key: str,
default_converter: Callable[[Date], Any] = to_j_instant,
) -> None:
"""
Expand All @@ -376,6 +411,7 @@ def convert_date_props(
callable_date_props: A set of callable date keys to convert.
The prop value should be a callable that takes a Date.
priority: The priority of the props to check.
granularity_key: The key for the granularity
default_converter: The default converter to use if none of the priority props are present.

Returns:
Expand All @@ -388,6 +424,16 @@ def convert_date_props(
# the simple props must be converted before this to simplify the callable conversion
converter = _prioritized_callable_converter(props, priority, default_converter)

# based on the convert set the granularity if it is not set
# Local Dates will default to DAY but we need to default to SECOND for the other types
if props.get(granularity_key) is None and converter != to_j_local_date:
props[granularity_key] = "SECOND"

# now that the converter is set, we can convert simple props to strings
for key in simple_date_props:
if props.get(key) is not None:
props[key] = str(props[key])

for key in callable_date_props:
if props.get(key) is not None:
if not callable(props[key]):
Expand Down
Loading
Loading