-
Notifications
You must be signed in to change notification settings - Fork 189
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 pyiterate #594
add pyiterate #594
Conversation
I'd suggest Base.iterate(o::PyObject) = Base.iterate(pyiterate(o, PyAny)) Iteration without auto-conversion can be done with for e in pyiterate(o, PyObject)
e :: PyObject
end We would need a helper sturct for this. Something like struct PyIterator{T}
o::PyObject
end
pyiterate(o::PyObject, eltype::DataType) = Pyiterator{eltype}(o)
Base.eltype(::Type{PyIterator{T}}) where T = T
Base.eltype(::Type{PyIterator{PyAny}}) = Any
Base.iterate(iter::PyIterator) = ... (Maybe we don't need the helper/factory function @stevengj What do you think? |
@tkf I like this proposal very much, and implemented it (without the helper function). |
We may want to generalize this to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. You are fast! I think it's good to go once Base.IteratorSize
definitions are added.
For getitem, you can already do julia> get(py"{'a': 1}"o, PyObject, "a")
PyObject 1 (which I just learnt recently). For getattr, you can kind of do it already as well: julia> pycall(pybuiltin("getattr"), PyObject, py"1"o, "__add__")
PyObject <method-wrapper '__add__' of int object at 0x7f8a4d3583e0> though actually it's shorter to write julia> pycall(py"lambda x: x.__add__", PyObject, py"1"o)
PyObject <method-wrapper '__add__' of int object at 0x7f8a4d3583e0> |
Thanks for these tips! I was not aware of |
Co-Authored-By: jw3126 <[email protected]>
Co-Authored-By: jw3126 <[email protected]>
Co-Authored-By: jw3126 <[email protected]>
Co-Authored-By: jw3126 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
It looks like the CI failures are network issue. Maybe due to https://blog.github.com/2018-10-21-october21-incident-report/
So can this be merged? Or is there any remaining problem in this PR? |
* switch to 0.7/1.0 properties * still support 0.6 * adjust runtests * updated tests * test fix (0.6) * modified tests * added getindex and haskey deprectations; fixed some internal warnings * starequal in pynothing comparisons * separate depwarns for getindex * fix tests introduced in #594 * fixing deprecations... * fixing deprecations again... * fixed all deprecation warnings * deprecate setindex! and fix warnings etc * first (insufficient) modification of README
This is what I proposed in #593