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

Support datetime.(date|datetime) as a SchemaBase parameter #3651

Closed
2 tasks done
Tracked by #3650
dangotbanned opened this issue Oct 22, 2024 · 0 comments · Fixed by #3653
Closed
2 tasks done
Tracked by #3650

Support datetime.(date|datetime) as a SchemaBase parameter #3651

dangotbanned opened this issue Oct 22, 2024 · 0 comments · Fixed by #3653

Comments

@dangotbanned
Copy link
Member

dangotbanned commented Oct 22, 2024

Target schema (core.DateTime)

core.DateTime / datetime.datetime

  • Order, and name of parameters differ
  • Milli instead of Micro seconds resolution
  • Doesn't really handle timezones, just a utc flag

See #3650 for detail

Current

from datetime import date, datetime

import altair as alt

window_stdlib = (
    datetime(2005, 1, 1).timestamp() * 1e3,
    datetime(2009, 1, 1).timestamp() * 1e3,
)
window_alt = alt.DateTime(year=2005), alt.DateTime(year=2009)

brush = alt.selection_interval(encodings=["x"], value={"x": window_stdlib})
brush = alt.selection_interval(encodings=["x"], value={"x": window_alt})

Goal

Directly addressing #3643, with a simple stdlib solution

window_stdlib = datetime(2005, 1, 1), datetime(2009, 1, 1)

Annotations

  • Update generated annotations to include datetime.(date|datetime)
    • Note: won't be operating in the same way as jsonschema_to_python_types
    • Implement a guard (.is_datetime()) in tools.schemapi.utils.SchemaInfo
      • Where the schema permits core.DateTime, always add these primitives
      • Found 58 occurences for r'[/<]DateTime[">]' in vega-lite-schema.json
    • Insert either
      • {"datetime.date", "datetime.datetime"}
      • or a new alias for that union
  • Passing suite (29c500c)
dangotbanned added a commit that referenced this issue Oct 25, 2024
…3653)

* feat: Support `datetime.(date|datetime)` as a `SchemaBase` parameter

#3651

* test: Adds `test_to_dict_datetime`

* feat: Reject non-UTC timezones

The [vega-lite docs](https://vega.github.io/vega-lite/docs/datetime.html) don't mention the `utc` property - but this restricts us to `utc` or `None`

* test(typing): Adds `test_to_dict_datetime_typing`

Annotation support will be complete once `mypy` starts complaining that these comments are unused

* fix(typing): Remove unused ignore

https://github.com/vega/altair/actions/runs/11460348337/job/31886851940?pr=3653

* feat(typing): Adds `Temporal` alias

* build: run `generate-schema-wrapper`

**Expecting to fail `mypy`**

#3653 (comment)

* fix(typing): Remove unused ignores

#3653 (comment), https://github.com/vega/altair/actions/runs/11461315861/job/31890019636?pr=3653

* fix: Use object identity for `utc` test instead of name

Adds two tests with "fake" utc timezones

* refactor(typing): Narrow `selection_(interval|point)(value=...)` types

- Tentative
- Copied from `core.SelectionParameter`

* refactor(typing): Utilize `PrimitiveValue_T`

#3656

* refactor(typing): Factor out `_LiteralValue` alias

This was incomplete as it didn't include `None`, but was serving the same purpose as the generated alias

* feat(typing): Accurately type `selection_(interval|point)(value=...)`

Added some docs to the new types, since what they describe is somewhat complex

References:
- https://vega.github.io/vega-lite/docs/selection.html#current-limitations
- https://vega.github.io/vega-lite/docs/param-value.html
- https://vega.github.io/vega-lite/docs/selection.html#project
- https://github.com/vega/altair/blob/5a6f70dc193f7ed9c89e6cc22e95c9d885167939/altair/vegalite/v5/schema/vega-lite-schema.json#L23087-L23118

Resolves:
- #3653 (comment)
- #3653 (comment)

* test(typing): Add (failing) `test_selection_interval_value_typing`

Need to fix this, but adding this first to demonstrate the issue.
Currently the way I've typed it prevents using different types for each encoding.

The goal is for each encoding to restrict types independently.
E.g. here `x` is **only** `date`, `y` is **only** `float`

* fix(typing): Correct failing `selection_interval` typing

#3653 (comment)

* test(typing): Improve coverage for `test_selection_interval_value_typing`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant