Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Type hints: Parts of folders "vegalite", "v5", and "utils" #2976
Type hints: Parts of folders "vegalite", "v5", and "utils" #2976
Changes from 18 commits
4146bba
cc17fa2
e748ce0
7feb6ab
dd5694a
e1693e5
474d693
3f2c220
cb2a8a8
570a358
a166ee0
d34d07d
30af9a2
4ef8f93
c6e866e
1261511
fa9c081
1b0d531
15f89fd
54c0f03
e974bc9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another question, here you mention:
data: Union[np.ndarray, pd.Series]
. When I look to where this is used: https://github.com/altair-viz/altair/blob/master/altair/vegalite/v5/api.py#L2411, I cannot understand why we need to be restrictive on the input types from numpy and pandas here. I really like types likeDataFrameLike
andSupportsGeoInterface
for input data features (so we can be flexible on input and restrictive on output).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to define a type hint named
ArrayLike1D
something that can serve for one-dimensional data of various forms, but not sure if this is possible yet (eg. python/mypy#12280).Otherwise, maybe something like they define as type hint for the
values
of a polars series? There they define anArrayLike
type as such (see here:And in the docstring for values of the series they mention:
I don't think this type-hint is strictly 1D as the docstring suggest, but it can serve as a reference.
Somehow wished it would be possible to include
range
as well (ref: #2877).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, that's too restrictive. I used the type hints which were already present in the docstring but seems like this function supports anything that the pandas function
infer_type
can handle. According to their own type hints this is allobject
so basically everything. I'd suggest that we type hint it the same to be consistent with pandas. Implemented in e974bc9Btw, the function
infer_vegalite_type
is used only here. Your link points to the usage ofinfer_encoding_types
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is flexible enough, thanks for the change.