JuliaPy / PythonCall.jl

Python and Julia in harmony.
https://juliapy.github.io/PythonCall.jl/stable/
MIT License
776 stars 63 forks source link

Allow multi-block expressions in `seval` #452

Closed MilesCranmer closed 7 months ago

MilesCranmer commented 7 months ago

Simpler than I thought!

Fixes https://github.com/JuliaPy/PythonCall.jl/issues/433

@LilithHafner @cjdoris

The only difference between these is that Meta.parse checks for multiple blocks and throws an error:

    ex, pos = parse(str, 1; filename, greedy=true, raise, depwarn)
    if isexpr(ex, :error)
        return ex
    end
    if pos <= ncodeunits(str)
        raise && throw(ParseError("extra token after end of expression"))
        return Expr(:error, "extra token after end of expression")
    end
    return ex

whereas Meta.parseall ignores the number of blocks:

    ex,_ = _parse_string(text, String(filename), lineno, 1, :all)
    return ex

Note that otherwise the behavior is identical.

LilithHafner commented 7 months ago

It's worth noting that Meta.parseall is undocumented, but this still looks like a good fix to me.

I suppose it would be possible to do this with public API by repeatedly parsing with the two-arg parse method and then manually joining them into a single expr, but I like this approach better.

MilesCranmer commented 7 months ago

Is it public since it is parseall rather than _parseall? (I don't know; just wondering). There are even some exported functions from Julia that don't have docs IIRC.

Just for posterity: I just tried to do a 2-arg Meta.parse version and ran into some sharp edges of the interface. For this reason I think parseall is safer even if not documented given it's unit-tested in Base and calls the same underlying _parse_str but with a different option.

cjdoris commented 7 months ago

Great thanks! Given Meta.parseall is used directly for code loading it seems unlikely to go away. Would be good to get it documented in Julia though, since there's no other documented reliable way to parse top-level expressions.