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

ENH: Change to CDN for base url #42

Merged
merged 5 commits into from
Jun 22, 2020

Conversation

eitanlees
Copy link
Contributor

resolves #41

@eitanlees
Copy link
Contributor Author

vega_datasets/__init__.py:5:1: E402 module level import not at top of file

So the build is failing flake8 because I moved SOURCE_TAG to the top of the __init__.py file. The reason I did this is so it can be referenced in core.py

What do you recomend?

We could change the test to ignore E402, or do something else to access the SOURCE_TAG. Maybe even more it into core.py?

@eitanlees
Copy link
Contributor Author

I went ahead and moved SOURCE_TAG into core.py.

I also modified generate_datasets_json.py such that it updates the correct file.

I think this should be working as expected now.

@jakevdp
Copy link
Member

jakevdp commented Jun 22, 2020

Looks good! To populate the new URLs within the package, run python tools/generate_datasets_json.py

Edit: nevermind, this is not necessary.

But it would be useful to run this just to make sure it works, since the tools are not covered by tests!

@eitanlees
Copy link
Contributor Author

yes, I checked that it worked locally

@jakevdp
Copy link
Member

jakevdp commented Jun 22, 2020

Thanks!

@jakevdp jakevdp merged commit dcd18ed into altair-viz:master Jun 22, 2020
@eitanlees eitanlees mentioned this pull request Jul 22, 2020
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.

Consider using CDN for the base url
2 participants