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

Fix 0.6 depwarns #356

Merged
merged 1 commit into from
Jan 30, 2017
Merged

Fix 0.6 depwarns #356

merged 1 commit into from
Jan 30, 2017

Conversation

yuyichao
Copy link
Collaborator

Test on master might fail with an error similar to

signal (4): Illegal instruction
while loading /home/yuyichao/projects/contrib/PyCall.jl/test/runtests.jl, in expression starting on line 318
copy at /usr/share/julia/site/v0.6/PyCall/src/numpy.jl:340
unknown function (ip: 0x7fe154c964f2)
jl_call_method_internal at /build/julia-git/src/julia-avx2/src/julia_internal.h:248

and that's JuliaLang/julia#20305

I also decided to work around JuliaLang/julia#19987 since it doesn't seems to have very high priority or trivial to fix (but trivial to workaround)....

@yuyichao
Copy link
Collaborator Author

The CI failure on 0.4 is caused by using the conditional comprehension syntax which exists on master.

@yuyichao
Copy link
Collaborator Author

0.4 syntax error fixed.

@yuyichao
Copy link
Collaborator Author

So the error doesn't occur on travis..... Maybe because numpy isn't installed? (It was triggered in a numpy specific test...)

function pyany_toany(T::Type)
T === Vararg{PyAny} ? Vararg{Any} : T
end
pyany_toany(::Type{PyAny}) = Any
Copy link
Member

Choose a reason for hiding this comment

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

Why was this changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Type{Vararg{PyAny}} isn't a valid type anymore.

Copy link
Member

Choose a reason for hiding this comment

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

Why not?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know the full story but it was never an actual type (you can't find a value that matches this type)

Copy link
Member

Choose a reason for hiding this comment

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

I'm confused, so why does T === Vararg{PyAny} still work if that is not a type?

Copy link
Collaborator Author

@yuyichao yuyichao Jan 29, 2017

Choose a reason for hiding this comment

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

The usage of it has always been strange. It is essentially only meaningful in a tuple but it's hard to only allow it there without breaking many other things. It is used to be allowed to use it just like a type though almost all types constructed this way are invalid. This is now disallowed.

It is still allowed to be used as a value and is still treated as a type for dispatch purpose even if it never actually was one. I believe the long term goal is to get rid of it or somehow being able to pass it around without dealing with this special case or generating invalid types.

Also fix 0.4 syntax error
@codecov-io
Copy link

codecov-io commented Jan 29, 2017

Current coverage is 64.59% (diff: 100%)

Merging #356 into master will increase coverage by 3.09%

@@             master       #356   diff @@
==========================================
  Files            15         15          
  Lines          1374       1384    +10   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            845        894    +49   
+ Misses          529        490    -39   
  Partials          0          0          

Powered by Codecov. Last update b9e59b5...f31953d

@stevengj stevengj merged commit af7213b into JuliaPy:master Jan 30, 2017
@yuyichao yuyichao deleted the 0.6 branch January 30, 2017 02:25
@malmaud
Copy link
Contributor

malmaud commented Jan 30, 2017

@stevengj Any objection to tagging PyCall now that we have this?

@stevengj
Copy link
Member

Tagged 1.9

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.

4 participants