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

switch to Project file #44

Merged
merged 4 commits into from
Feb 17, 2020
Merged

switch to Project file #44

merged 4 commits into from
Feb 17, 2020

Conversation

piever
Copy link
Contributor

@piever piever commented Feb 15, 2020

I've put the (previously unspecified) Tables dependency directly to 1.0 (I need it to have StatsMakie compatibility with Tables 1.0).

The only thing I don't understand is a NamedArray method error in one of the tests. I've commented it out for the moment.

@@ -168,7 +168,7 @@ for docat in [false, true]
@test tab == [2 0
0 5
1 6]
@test names(tab[1:2, :]) == [["Iris-setosa", "Iris-versicolor"], [false, true]]
# @test names(tab[1:2, :]) == [["Iris-setosa", "Iris-versicolor"], [false, true]]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This only errors for docat == true (see for loop above). It's an unrelated method error, not sure where it comes from, but I suspect it has something to do with CategoricalArrays.

Copy link
Owner

@nalimilan nalimilan Feb 17, 2020

Choose a reason for hiding this comment

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

Good catch. It also happens on master so that's not a blocker for this PR -- let's keep this change out for now.

Here's a reproducer:

using NamedArrays, CategoricalArrays
NamedArray(ones(3, 2), (["a", "b", "c"], Vector(categorical([true, false]))), ("A", "B"))[1:2,:]

This is a corner case which happens when passing a Vector{<:CategoricalValue}, which gets converted to a CategoricalVector on indexing, which apparently isn't supported by NamedArrays. This only happens when crossing categorical and non categorical variables. The code should unwrap the values.

Filed #45.

@coveralls
Copy link

Coverage Status

Coverage increased (+3.0%) to 95.714% when pulling 21717bb on piever:pv/project into 1bc6c71 on nalimilan:master.

@piever
Copy link
Contributor Author

piever commented Feb 15, 2020

It also seems that Tables.jl requires julia 1.0 compatibility. Is it OK to drop julia 0.7 compatibility here?

@nalimilan
Copy link
Owner

Yes I think it's fine to require Julia 1.0.

@nalimilan
Copy link
Owner

Tests are failing everywhere with the right error, so merging. :-)

@nalimilan nalimilan merged commit f690370 into nalimilan:master Feb 17, 2020
@piever piever deleted the pv/project branch February 17, 2020 16:42
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