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

MAINT: multiple vega-lite versions within Altair #377

Merged
merged 8 commits into from
Oct 2, 2017

Conversation

jakevdp
Copy link
Collaborator

@jakevdp jakevdp commented Aug 16, 2017

This PR rearranges some things to allow multiple versions of Vega-Lite to be supported in a single Altair release (addresses #375). So, for example, you could do as normal

import altar as alt

alt.Chart(data).mark_point().encode( ... )

or equivalently,

import altair.v1 as alt

alt.Chart(data).mark_point().encode( ... )

Currently v1 is the only option available, but this reorganization of the code will make it easy to add additional versions as well.

@jakevdp
Copy link
Collaborator Author

jakevdp commented Aug 16, 2017

Changes look big, but the bulk of it is just rearranging existing code.

@jakevdp
Copy link
Collaborator Author

jakevdp commented Aug 16, 2017

Tests pass! @ellisonbg, would you mind taking a quick look? One thing I'm not certain on is the mirroring of altair.v1 submodules in the altair namespace. Does the way I did this make sense to you?

@ellisonbg
Copy link
Collaborator

Working on reviewing this:

We have an example notebook that looks at api.AggregateOp.values, but AggregateOp is no longer in api. Do we want this to be in the public API. I am fine removing it from the examples or exporting it in api.

I am seeing a test failure locally:

======================================================== FAILURES =========================================================
_________________________________________________ test_hastraits_required _________________________________________________

    def test_hastraits_required():
    
        class Foo(jst.JSONHasTraits):
            _required_traits = ['name']
            name = jst.JSONString()
            age = jst.JSONNumber()
    
        f1 = Foo(name="Sue", age=32)
        f2 = Foo(age=32)
    
        # contains all required pieces
        f1.to_dict()
    
        with pytest.raises(T.TraitError) as err:
            f2.to_dict()
>       assert err.match("Required trait 'name' is undefined")
E       AttributeError: 'ExceptionInfo' object has no attribute 'match'

altair/v1/schema/_interface/tests/test_jstraitlets.py:132: AttributeError
===================================== 1 failed, 381 passed, 4 skipped in 8.57 seconds =====================================

@jakevdp you were asking about mirroring the sub are you asking about how you are importing expr and tutorial in v1.__init__. If we want to recommend people to do the following imports, then I think we need to:

import altair as alt
import altair.v1 as alt

So +1 on that unless I am misunderstanding something.

@ellisonbg ellisonbg added this to the 1.2.1 milestone Sep 27, 2017
@ellisonbg
Copy link
Collaborator

Going to merge and iterate...we can make adjustments later if needed.

@ellisonbg ellisonbg merged commit cf137fc into vega:master Oct 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants