-
Notifications
You must be signed in to change notification settings - Fork 795
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: Adds ChartType
type and type guard is_chart_type
. Change PurePath to Path type hints
#3420
Commits on May 12, 2024
-
feat: Type hints: Adds
ChartType
and guardis_chart_type
I'm working on a library that uses `altair`, and looking to make it an optional dependency. During this process, I thought `ChartType` and `is_chart_type` might be a good fit in `altair` itself. I don't believe any existing `altair` code would directly benefit from these additions - but you could likely reduce some `isinstance` checks with [type narrowing](https://typing.readthedocs.io/en/latest/spec/narrowing.html) functions like `is_chart_type`. For example `altair/vegalite/v5/api.py` has **63** occurrences of `isinstance`. --- per [CONTRIBUTING.md](https://github.com/vega/altair/blob/main/CONTRIBUTING.md) > In particular, we welcome companion efforts from other visualization libraries to render the Vega-Lite specifications output by Altair. We see this portion of the effort as much bigger than Altair itself Towards this aim, it could be beneficial to provide `typing` constructs/tools for downstream use. Regarding my specfic use case, the below [Protocol](https://typing.readthedocs.io/en/latest/spec/protocol.html#runtime-checkable-decorator-and-narrowing-types-by-isinstance) - I believe - describes the concept of a `_/Chart`, for the purposes of writing to file: ```py from typing import Any, Protocol, runtime_checkable @runtime_checkable class AltairChart(Protocol): data: Any def copy(self, *args: Any, **kwargs: Any) -> Any: ... def save(self, *args: Any, **kwargs: Any) -> None: ... def to_dict(self, *args: Any, **kwargs: Any) -> dict[Any, Any]: ... def to_html(self, *args: Any, **kwargs: Any) -> str: ... ``` Due to the number of symbols in `altair` and heavy use of mixins, coming to this conclusion can be a stumbling block. However, exporting things like `ChartType`, `is_chart_type`, `AltairChart` could be a way to expose these shared behaviours - without requiring any changes to the core API.
Configuration menu - View commit details
-
Copy full SHA for 6964d0d - Browse repository at this point
Copy the full SHA 6964d0dView commit details -
fix: Type hints: Add missing
pathlib.Path
insave
method annotation`TopLevelMixin.save` calls a function accepting `pathlib` objects, but the two didn't share the same `Union` for `fp`. When using `pyright`, the following warning is presented: > Argument of type "Path" cannot be assigned to parameter "fp" of type "str | IO[Unknown]" in function "save" > Type "Path" is incompatible with type "str | IO[Unknown]" > "Path" is incompatible with "str" > "Path" is incompatible with "IO[Unknown]" This fix avoids the need to use type ignores on the public facing API, as a user.
Configuration menu - View commit details
-
Copy full SHA for 96bf092 - Browse repository at this point
Copy the full SHA 96bf092View commit details -
refactor: Simplify
save.py
path handlingBy normalizing `fp` in `save`, it simplifies the signatures of two other functions and removes the need for `str` specific suffix handling.
Configuration menu - View commit details
-
Copy full SHA for 9c770a5 - Browse repository at this point
Copy the full SHA 9c770a5View commit details -
refactor: Replace duplicated
save
get encoding with a single expres……sion Although there are branches it isn't needed, I think this is easier to read and maintain. Very low priority change, just something I noticed while reading.
Configuration menu - View commit details
-
Copy full SHA for 99b4f81 - Browse repository at this point
Copy the full SHA 99b4f81View commit details -
Configuration menu - View commit details
-
Copy full SHA for 55cddd5 - Browse repository at this point
Copy the full SHA 55cddd5View commit details
Commits on May 13, 2024
-
Configuration menu - View commit details
-
Copy full SHA for a97f760 - Browse repository at this point
Copy the full SHA a97f760View commit details -
Configuration menu - View commit details
-
Copy full SHA for 32010bb - Browse repository at this point
Copy the full SHA 32010bbView commit details -
build: Make new additions to
api
privateBelieve these should be public, but it seems like the issue at build-time is that I'd be extending the public API.
Configuration menu - View commit details
-
Copy full SHA for 14bd0e6 - Browse repository at this point
Copy the full SHA 14bd0e6View commit details
Commits on May 15, 2024
-
Configuration menu - View commit details
-
Copy full SHA for 8a7bada - Browse repository at this point
Copy the full SHA 8a7badaView commit details
Commits on May 20, 2024
-
Configuration menu - View commit details
-
Copy full SHA for 8dd2cde - Browse repository at this point
Copy the full SHA 8dd2cdeView commit details -
Make ChartType and is_chart_type public. Use typing.get_args to reduc…
…e duplication of list of charts
Configuration menu - View commit details
-
Copy full SHA for a4de6d1 - Browse repository at this point
Copy the full SHA a4de6d1View commit details -
Configuration menu - View commit details
-
Copy full SHA for f12aea0 - Browse repository at this point
Copy the full SHA f12aea0View commit details
Commits on May 21, 2024
-
refactor: Adds ruff
SIM101
and apply fixesAs [requested](https://github.com/vega/altair/pull/3420/files/a4de6d1feb6f3f17c7f6d4c7738eb008ff33d17e#r1606780541). Seems to only occur in a few places, but all were autofix-able.
Configuration menu - View commit details
-
Copy full SHA for 9b8f6a8 - Browse repository at this point
Copy the full SHA 9b8f6a8View commit details -
revert: Change
set
back tolist
forformat
As [requested](vega@99b4f81#r1606785466)
Configuration menu - View commit details
-
Copy full SHA for 19010ba - Browse repository at this point
Copy the full SHA 19010baView commit details -
Configuration menu - View commit details
-
Copy full SHA for 082cd60 - Browse repository at this point
Copy the full SHA 082cd60View commit details -
Configuration menu - View commit details
-
Copy full SHA for 66a07d4 - Browse repository at this point
Copy the full SHA 66a07d4View commit details
Commits on May 23, 2024
-
revert: Add
str
support back for "private"save
functionsAn earlier commit assumed a minor optimization would be possible, by normalizing a `Union` early and reuse the narrowed type in "private" functions. @binste [comment](https://github.com/vega/altair/pull/3420/files#r1610440552)
Configuration menu - View commit details
-
Copy full SHA for fad765b - Browse repository at this point
Copy the full SHA fad765bView commit details