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

Fix Altair import in tools scripts #2872

Merged
merged 3 commits into from
Feb 4, 2023
Merged

Conversation

binste
Copy link
Contributor

@binste binste commented Feb 4, 2023

The current importing behaviour in the tools scripts has some inconsistencies which this PR tries to address. These issues were partially discovered in #2814.

Issue 1: Altair is imported for generate_api_docs before the vegalite files are updated

The statement import generate_api_docs was previously at the top of generate_schema_wrapper.py. In generate_api_docs.py, one of the first statements is the import of Altair -> Altair got imported before generate_schema_wrapper can update the vegalite files. This was presumably not a big issue so far as this script is usually run multiple times anyway and so the api docs were updated eventually.

This PR solves this by moving the import generate_api_docs statement at the end of the main function in generate_schema_wrapper.py.

Issue 2: If Altair is installed as a site-package, it might be imported instead of the currently checked out version

If you execute python tools/generate_api_docs.py then Python correctly uses Altair from the root folder due to the modification of sys.path in the beginning of the script. However, if generate_api_docs is improted from generate_schema_wrapper, Python seems to use sys.path as it's defined in generate_schema_wrapper for the whole import procedure, i.e. sys.path needs to be modified to be able to import Altair before generate_api_docs is imported.

You can check this by installing Altair in site-packages with pip and then adding the following lines in generate_api_docs.py right after the Altair import:

print("--------")
print(alt.__file__)
print("--------")

On the current master branch, when I execute python tools/generate_schema_wrapper.py I get the path to my site-packages Altair:

--------
/home/vscode/.local/lib/python3.11/site-packages/altair/__init__.py
--------

When I execute python tools/generate_api_docs.py I get the path to the correct local version in the current folder (/workspaces/altair):

--------
/workspaces/altair/altair/__init__.py
--------

This PR solves this by adding the root folder to sys.path already in generate_schema_wrapper.py so it doesn't matter anymore if you call python tools/generate_schema_wrapper.py or python tools/generate_api_docs.py. Both now import the correct local version of Altair.

@mattijn
Copy link
Contributor

mattijn commented Feb 4, 2023

I've test this PR and can confirm it is now working correctly. I've been wondering before why the api docs were different when generated from within the generate_schema_wrapper.py vs generating it directly using generate_api_docs.py. Merging. Thanks @binste!

@mattijn mattijn merged commit 79986bb into vega:master Feb 4, 2023
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.

2 participants