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

add pyiterate #594

Merged
merged 17 commits into from
Nov 4, 2018
Merged

add pyiterate #594

merged 17 commits into from
Nov 4, 2018

Conversation

jw3126
Copy link
Contributor

@jw3126 jw3126 commented Oct 16, 2018

This is what I proposed in #593

@tkf
Copy link
Member

tkf commented Oct 16, 2018

I'd suggest pyiterate to behave like pycall(o::PyObject, returntype::TypeTuple), i.e., pyiterate(o::PyObject, eltype::DataType) returns a Julia iterator with conversion to the given eltype. The default Base.iterate then would be defined as

Base.iterate(o::PyObject) = Base.iterate(pyiterate(o, PyAny))

Iteration without auto-conversion can be done with pyiterate(o, PyObject):

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 pyiterate and should expose PyIterator as an API.)

@stevengj What do you think?

@jw3126
Copy link
Contributor Author

jw3126 commented Oct 17, 2018

@tkf I like this proposal very much, and implemented it (without the helper function).

@jw3126
Copy link
Contributor Author

jw3126 commented Oct 17, 2018

We may want to generalize this to PyAccessor{T} that gives control over conversation not only for iteration, but also for getitem and getattr. What do you think?

Copy link
Member

@tkf tkf left a 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.

src/pyiterator.jl Show resolved Hide resolved
@tkf
Copy link
Member

tkf commented Oct 17, 2018

getitem and getattr

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>

@jw3126
Copy link
Contributor Author

jw3126 commented Oct 17, 2018

Thanks for these tips! I was not aware of get either, nice!

src/pyiterator.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
src/pyiterator.jl Show resolved Hide resolved
src/pyiterator.jl Outdated Show resolved Hide resolved
src/pyiterator.jl Outdated Show resolved Hide resolved
src/pyiterator.jl Outdated Show resolved Hide resolved
Copy link
Member

@tkf tkf left a 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/

src/pyiterator.jl Outdated Show resolved Hide resolved
@jw3126
Copy link
Contributor Author

jw3126 commented Nov 2, 2018

So can this be merged? Or is there any remaining problem in this PR?

@stevengj stevengj merged commit 8eb62f2 into JuliaPy:master Nov 4, 2018
carstenbauer added a commit to carstenbauer/PyCall.jl that referenced this pull request Jan 22, 2019
stevengj pushed a commit that referenced this pull request Jan 23, 2019
* 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
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