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

Add the altair package #4580

Merged
merged 4 commits into from
Feb 29, 2024
Merged

Add the altair package #4580

merged 4 commits into from
Feb 29, 2024

Conversation

joelostblom
Copy link
Contributor

Description

This adds the altair packages and closes #4579. I wasn't sure what tests would be appropriate, it seems like some package like mpl, pandas, etc tests very specific things whereas others such as shapely, bokeh, statsmodels, etc keep it more minimal. I started minimal for now but happy to add more if there are issues.

I couldn't test the actual graphical output via console.html and when I tried saving to a file via chart.save('chart.html') no errors were raised but I couldn't find the file anywhere (maybe it is saved to a temporary/virtual filesystem in the browser?).

Checklists

  • Add a CHANGELOG entry
  • Add / update tests
  • Add new / update outdated documentation

Copy link
Member

@hoodmane hoodmane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @joelostblom looks good to me.

@hoodmane
Copy link
Member

CI looks good so I'll go ahead and merge this.

@hoodmane
Copy link
Member

it seems like some package like mpl, pandas, etc tests very specific things whereas others such as shapely, bokeh, statsmodels, etc keep it more minimal. I started minimal for now but happy to add more if there are issues.

That's generally the way we work. Once upon a time a bunch of pandas things were broken, but we upstreamed all the patches and it hasn't caused any trouble in a long time =D

@joelostblom
Copy link
Contributor Author

Great, thanks @hoodmane ! One last thing to note is that some of the dependencies to altair have a version restriction (e.g. pandas >= 0.25), but I didn't find a way to add that in the meta.yaml and when I looked at other packages, it seems like they don't specify version restriction although their conda feedstock recipes indicate specific versions (e.g. statsmodels conda feedstock vs pyodide yaml)

@hoodmane
Copy link
Member

Yeah we only have one version of each package. We currently don't check constraints and just pray that nothing goes wrong. But there would be an easy way to validate the collection of versions, see #4570.

@hoodmane
Copy link
Member

@joelostblom CI looks good, should I merge?

@joelostblom
Copy link
Contributor Author

Yes, merge away! There is nothing else from my side I believe, and it seems to be working as it should based on my testing console.html (except for the note about not finding the saved file in my original comment).

@hoodmane
Copy link
Member

@hoodmane hoodmane merged commit 2731e22 into pyodide:main Feb 29, 2024
39 of 41 checks passed
@hoodmane
Copy link
Member

Thanks @joelostblom!

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.

Package request: Altair
2 participants