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

use 'UndefinedLike = Any' as the type hint #2681

Merged
merged 4 commits into from
Oct 13, 2022

Conversation

brahn
Copy link
Contributor

@brahn brahn commented Sep 20, 2022

Improvement on #2670 -- see summary and discussion there

@mattijn
Copy link
Contributor

mattijn commented Sep 20, 2022

As is mentioned in this comment: #2614, which counts here as well:

Thanks! Note the line at the top of the file: https://github.com/altair-viz/altair/blob/c4248a953c8518efb3cf5bef8d9755bd0fc7391e/altair/utils/schemapi.py#L1-L2

You should make these changes in https://github.com/altair-viz/altair/blob/master/tools/schemapi/schemapi.py instead, and then run generate_schema_wrapper.py to sync the results to this file.

@brahn
Copy link
Contributor Author

brahn commented Sep 21, 2022

Thanks @mattijn ! I believe I've proceeded as you described.

I don't know the origin of the (small) changes the script made to API.rst since they don't seem related to my schema change, but I'm guessing that they are either the result of some prior changes for which generate_schema_wrapper.py wasn't run, or a slight version change in some of the dependencies.

Also I should mention that when I first tried to run generate_schema_wrapper.py I encountered a bug in the m2r library described here, which I resolved by using the patch helpfully offered in that post.

@brahn
Copy link
Contributor Author

brahn commented Sep 28, 2022

@mattijn @jakevdp just checking back in, please let me know if there's anything I can do to help!

@mattijn
Copy link
Contributor

mattijn commented Sep 28, 2022

I've no opinion on this.

I hope others can review this.

If there will not appear any reviewers nor disapprovals I will merge this in two weeks from now.

@joelostblom
Copy link
Contributor

I don't have enough expertise to review this properly unfortunately.

@mattijn mattijn merged commit b186a76 into vega:master Oct 13, 2022
@brahn
Copy link
Contributor Author

brahn commented Oct 13, 2022

thanks @mattijn !

@brahn
Copy link
Contributor Author

brahn commented Oct 14, 2022

@mattijn quick follow-up -- forgive my ignorance, how does this factor into the version-release cycle?

@joelostblom
Copy link
Contributor

joelostblom commented Oct 14, 2022

It is currently not clear when the next release will be. Work is being done (most notably #2684) to have Altair support the latest features in VegaLite 5 and this will likely results in a major release eventually. Although I could see an argument for making a minor release that fixes some package compatibility issues and deprecations (e.g. #2650 #2683 #2686), I think we also want to focus our efforts so I am not sure that will happen.

@brahn
Copy link
Contributor Author

brahn commented Oct 14, 2022

@joelostblom understood, thx!

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.

3 participants