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

pyconvert_add_rule() ignored if cache has been populated #336

Closed
hhaensel opened this issue Jul 5, 2023 · 5 comments · Fixed by #365
Closed

pyconvert_add_rule() ignored if cache has been populated #336

hhaensel opened this issue Jul 5, 2023 · 5 comments · Fixed by #365
Labels
bug Something isn't working priority Should be fixed or implemented soon

Comments

@hhaensel
Copy link

hhaensel commented Jul 5, 2023

Currently, if I define a new rule from the beginning, everything works fine:

using PythonCall
using DataFrames

pdf = pytable(DataFrame(:a => 1:2, :b => 2:3))

PythonCall.pyconvert_rule_pandasdataframe(::Type{DataFrame}, x::Py) = DataFrame(PyTable(x))
PythonCall.pyconvert_add_rule("pandas.core.frame:DataFrame", DataFrame, PythonCall.pyconvert_rule_pandasdataframe, PythonCall.PYCONVERT_PRIORITY_ARRAY)

pyconvert(Any, pdf)

results in

2×2 DataFrame       
 Row │ a      b     
     │ Int64  Int64 
─────┼──────────────
   11      2
   22      3

However, if I do conversion once before the rule is defined, the conversion rules are cached and no conversion is performed. So far that's expected. But even if I delete the rule cache, the new rule is not applied.

using PythonCall
using DataFrames

pdf = pytable(DataFrame(:a => 1:2, :b => 2:3))

pyconvert(Any, pdf)

PythonCall.pyconvert_rule_pandasdataframe(::Type{DataFrame}, x::Py) = DataFrame(PyTable(x))
PythonCall.pyconvert_add_rule("pandas.core.frame:DataFrame", DataFrame, PythonCall.pyconvert_rule_pandasdataframe, PythonCall.PYCONVERT_PRIORITY_ARRAY)

empty!(PythonCall.PYCONVERT_RULES_CACHE)

pyconvert(Any, pdf)

results in

2×2 PyPandasDataFrame
   a  b
0  1  2
1  2  3

The reason is that somehow @generated pyconvert_rules_cache does not recalculate the value after the cache has been emptied.

julia> PythonCall.pyconvert_rules_cache(Any)
Dict{Ptr{PythonCall.C.PyObject}, Vector{Function}} with 1 entry:
  Ptr{PyObject} @0x00000219a6cf69e0 => [#54, #54, #54, #54, #54, #54, #54, #54, #54, #54, #54]

If I omit the @generated in front of pyconvert_rules_cache, the new rules are considered.

If this is by design in order to gain performance, it's perhaps worth mentioning in the docs of pyconvert_add_rule(). Otherwise removing @generated in convert.jl could be a solution.

@hhaensel hhaensel added the bug Something isn't working label Jul 5, 2023
@hhaensel
Copy link
Author

hhaensel commented Jul 5, 2023

Another posibility would be offering a resetting function

function pyconvert_reset_cache!()
    empty!(PYCONVERT_RULES_CACHE)
    Core.eval(@__MODULE__, quote
        @generated pyconvert_rules_cache(::Type{T}) where {T} = get!(Dict{C.PyPtr, Vector{Function}}, PYCONVERT_RULES_CACHE, T)
    end)
end

@github-actions
Copy link
Contributor

This issue has been marked as stale because it has been open for 30 days with no activity. If the issue is still relevant then please leave a comment, or else it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues about to be auto-closed label Aug 19, 2023
@cjdoris
Copy link
Collaborator

cjdoris commented Aug 21, 2023

Hi, I'll take a look. AFAIR calling pyconvert_add_rule should reset caches already, but maybe one of the later-stage caches is not being cleared.

@hhaensel
Copy link
Author

hhaensel commented Aug 21, 2023

Hi, just read that you are looking into this.
Meanwhile I have defined two functions for myself:

function pyconvert_reset_cache!()
    empty!(PythonCall.PYCONVERT_RULES_CACHE)
    Core.eval(PythonCall, quote
        @generated pyconvert_rules_cache(::Type{T}) where {T} = get!(Dict{C.PyPtr, Vector{Function}}, PYCONVERT_RULES_CACHE, T)
    end)
end

function pyconvert_reset!()
    empty!(PythonCall.PYCONVERT_RULES)
    empty!(PythonCall.PYCONVERT_EXTRATYPES)
    pyconvert_reset_cache!()
    PythonCall.init_pyconvert()
end

@github-actions github-actions bot removed the stale Issues about to be auto-closed label Aug 22, 2023
@github-actions

This comment was marked as outdated.

@github-actions github-actions bot added the stale Issues about to be auto-closed label Sep 21, 2023
@cjdoris cjdoris added priority Should be fixed or implemented soon and removed stale Issues about to be auto-closed labels Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority Should be fixed or implemented soon
Projects
None yet
2 participants