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

Make transformed_data public and add initial docs #3084

Merged
merged 6 commits into from
Jun 16, 2023

Conversation

jonmmease
Copy link
Contributor

This is a follow-on to #3081 that makes the new _transformed_data chart methods public and adds an initial documentation section to https://altair-viz.github.io/user_guide/data_transformers.html.

I didn't find an existing sphinx extension to evaluate a Python snippet that doesn't produce an Altair chart, so I added the ipython sphinx directive for this purpose. I'm happy to consider other options if folks have other ideas.

There are probably other places where it would be good to mention this functionality, but this is a starting point. To other maintainers, feel free to commit suggestions directly to this branch, or leave them as review comments.

Thanks!

Copy link
Contributor

@binste binste left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Contributor

@joelostblom joelostblom left a comment

Choose a reason for hiding this comment

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

This looks good to me. I am surprised that an IPython directive is needed to evaluate the code for non-altair output. There are a couple of spaces in the "Specifying data" section where it seems like we show output without using this directive, for example here:

image

I don't have time to play around with this myself right now, and I'm not against adding more directives to the documentation so I'm happy for this PR to be merged as is, but thought I would just mention the above in case it is helpful.

doc/user_guide/transform/index.rst Show resolved Hide resolved
@jonmmease
Copy link
Contributor Author

There are a couple of spaces in the "Specifying data" section where it seems like we show output without using this directive

Thanks for calling that out, I'll take a look at how this works and report back.

@jonmmease
Copy link
Contributor Author

Thanks for calling that out @joelostblom, it works with

.. altair-plot::             
    :output: repr            
                             
    import altair as alt
    ...

I had missed the :output: repr option. ipython directive removed in be59e62

Copy link
Contributor

@joelostblom joelostblom left a comment

Choose a reason for hiding this comment

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

Great, merging!

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