Shortcut macros for collections

[daniil-konovalenko]

This is an implementation of #1463
It currently provides the following macros:

• py_list!(py, [1, 2, 3]) → &PyList
• py_tuple!(py, (1, 2, 3)) → &PyTuple
• py_dict!(py, {"key": value}) → PyResult<&PyDict>
• py_set!(py, {"item1", "item2", 1, 2, 3}) → PyResult<&PySet>
• py_frozenset!(py, {"item1", "item2", 1, 2, 3}) → PyResult<&PyFrozenSet>

py_dict! only supports literals as keys because we use colons : as key-value separators, and colons are not allowed after expr fragments. If we would like to use expressions as keys, we have to use => as a separator.

TODO:

• : vs => for key-value separator
• proc macro for py_dict!
• PyDict::from_sequence doesn't exist on PyPy
• an entry in CHANGELOG.md
• docs to all new macros and details in the guide
• more tests probably

[davidhewitt]

Very cool, thanks for working on this! Let's take some time to get this right; we can merge it after the 0.14 release so that there's plenty of time to make the right compromises.

py_dict! only supports literals as keys because we use colons : as key-value separators, and colons are not allowed after expr fragments. If we would like to use expressions as keys, we have to use => as a separator.

Ah, that's really frustrating. There is precedent for => from the maplit::hashmap! macro, though it's a bit of a shame to deviate from the python syntax. Perhaps as well as literals, we could allow idents? Or I wonder if we could use a proc macro for py_dict! to allow us to have exactly the syntax we want. (proc macro in expression position is possible from Rust 1.42 if I recall correctly; I think it's fine if these macros don't work on MSRV.)

I'd be interested in hearing some other maintainer's opinions on the design considerations here, as the way they're implemented at the moment is basically exactly the way I suggested in #1463.

One final syntax idea I have is that we could consider using py => at the beginning, e.g.

py_list!(py => [1, 2, 3]);

I saw this in the quote::quote_spanned! macro and quite liked it. Wonder what you think of it?

[daniil-konovalenko]

Thank you for such a quick reaction!

Let's take some time to get this right

Absolutely! Let's try to make this as pleasant to use as possible.

Perhaps as well as literals, we could allow idents?

Sure, idents shouldn't be a problem. A proc macro sounds even better, I did not know it was an option here, thanks for the hint! I will see what we can do with it.

I'd be interested in hearing some other maintainer's opinions on the design considerations here, as the way they're implemented at the moment is basically exactly the way I suggested in #1463.

Me too!
One thing that bugs me at the moment is that deeply nested collections look really convoluted because of all the py's and result handling:

let my_complex_dict = py_dict!(py, {
    "username": "admin",
    "permissions": py_set!(py, {"read", "write"}).unwrap(),
    "default_args": py_tuple!(py, (py_list!(py, [1, "order_id", py_dict!(py, {"meta": py_list!(py, ["abcd", "efg"])}).unwrap()])))
}).unwrap();

compared to python

my_complex_dict = {
    'default_args': (
        [1, 'order_id', {'meta': ['abcd', 'efg']}],
    ),
    'permissions': {'read', 'write'},
    'username': 'admin',
}

I wonder if we can do better here.

One final syntax idea I have is that we could consider using py => at the beginning, e.g.
py_list!(py => [1, 2, 3]);

I like it! It makes for a nice distinction between py and the actual collection items.

[davidhewitt]

One thing that bugs me at the moment is that deeply nested collections look really convoluted because of all the py's and result handling:

Hmm, that's true. There could be some kind of py_collection! macro which had all of the syntaxes in one, though I'm not sure I love that. I'd be open to hearing suggestions on this.

[mejrs]

I love this, thanks ❤️

One thing that bugs me at the moment is that deeply nested collections look really convoluted because of all the py's and result handling:

I think this is also because nested objects are convoluted.

Hmm, that's true. There could be some kind of py_collection! macro which had all of the syntaxes in one, though I'm not sure I love that.

This sounds pretty complicated to use. I can't say I've ever had a good time making/manipulating complex "stringly typed" structures. So I don't think this is something we should encourage.

I'd be open to hearing suggestions on this.

People can just define nested Rust structs and derive ToPyObject for it? Is this something we could make a derive macro for?

[davidhewitt]

People can just define nested Rust structs and derive ToPyObject for it? Is this something we could make a derive macro for?

Yes definitely. I think at the time that #[derive(FromPyObject)] was implemented there were some design questions about the to-python direction (e.g. ToPyObject or IntoPy), but it's very much something I'd support having.

I think you're right that that would be a cleaner way to create a deeply-nested object.

result handling

Just noticed this item. Do we expect many errors here? It might be nice to panic on invalid input to avoid the error handling, but it depends on how easy it is to write invalid code.

[daniil-konovalenko]

I think this is also because nested objects are convoluted.

I agree, creating deeply nested objects is probably not the intended use of these macros. I really like the #[derive(IntoPyObject)] idea though.

Do we expect many errors here?

I was thinking about passing unhashable objects as dict keys or set elements. I'm not sure how common that might be, so I made py_set! and py_dict! return Result for now. Perhaps we could panic on invalid input by default but also provide an alternative that returns Result instead, like try_py_dict!.

[davidhewitt]

I was thinking about passing unhashable objects as dict keys or set elements.

That's a fair point. I still think that even if users make an error here, it's pretty much hard-coded into the program with the macro, so it'll be fixed pretty quick after they see a panic.

I made py_set! and py_dict! return Result for now

To avoid suprising users I would suggest making either all of the macros of none of them return Result, so that they're not repeatedly having to remember the special cases.

I'm not particularly convinced we would need non-panicking try_ variants - when the user is unsure about the input, they can just use normal methods.

[davidhewitt]

Also - prior art on complex "collections" is the serde_json::json! macro.

Though there is also the inline_python crate which exposes the full Python syntax, which is probably a better way to do such a thing than us trying to make a py_collection! macro.

[birkenfeld]

I'm in favor or the non-panicking variant - the case of unhashable objects is one of those instances where everything works perfectly in tests, and then somebody passes an unexpected object, and poof. Let the user handle that case and panic if they want. It's not like memory allocation where you can consistently expect a panic from Rust.

(I also think py_run! should not panic; it's pretty useful but hampered by that behavior.)

[daniil-konovalenko]

Update:

I took a long break, but now I'm back!

I have recently figured out how to allow expressions in dict literal macros, thanks to @dtolnay's awesome proc-macro workshop and dtolnay/syn#704.
We now allow using expressions as dict keys and values:

Python::with_gil(|py| {
    let my_dict = py_dict!(py => {"KeY".to_lowercase(): 7 + 10}).unwrap();
    py_run!(py, my_dict, "assert my_dict['key'] == 17");
})

I have also implemented the py => {...} syntax that @davidhewitt suggested and it looks really nice!
I also discovered that, technically, we can ditch the requirement to pass py explicitly and rely on it being in scope. Although it makes for nicer syntax (let dict = py_dict!{"key": "value"}), I'm not sure if it's a good idea to be implicit here. Let me know your thoughts on this!

The code for py_dict! is pretty rough still, I am just excited to share that it works :)
I'll take some time to clean it up, write tests, make errors nicer, etc., in the meantime, please feel free to review the current implementation or wait until it's more ready — I'll appreciate any comments or suggesstions!

[davidhewitt]

Awesome! I've been quite busy recently so don't wait for my review, although I will definitely try to give one soon.

"KeY".to_lowercase(): 7 + 10}

Wow, I didn't even realise this was valid Python syntax! I confirmed it is indeed 😅

>>> {"KeY".lower(): 7 + 10}
{'key': 17}

I also discovered that, technically, we can ditch the requirement to pass py explicitly and rely on it being in scope. Although it makes for nicer syntax (let dict = py_dict!{"key": "value"}), I'm not sure if it's a good idea to be implicit here. Let me know your thoughts on this!

Yeah, I was thinking on this. The awesome thing about macros is that actually we can support both. As long as it's clearly documented, this seems ok to me. Something like:

py_dict!{ key: value }  // Assumes `py: Python` is already in scope
py_dict!( other_py => { key: value })  // To pass `other_py: Python` explicitly

I'd be interested to hear whether everyone else thinks that's an acceptable idea, or if we should aim to choose just one syntax.

I think if we decided to pick just one, I'd go for py_dict!( py=> { key: value }) for clarity.

[daniil-konovalenko]

The awesome thing about macros is that actually we can support both

It didn't even come to me, thanks for pointing that out! Let's see what others think of it :)

[mejrs]

I prefer the original syntax, e.g. py_list!(py, [1, 2, 3]). I don't get the meaning of this => syntax.

we should aim to choose just one syntax.

I agree

we can ditch the requirement to pass py explicitly and rely on it being in scope

What kind of errors do you get if you do this but Python is not in scope? Also this relies on it being named py, of which I'm not a fan.

[daniil-konovalenko]

Hi everyone!

I'm sorry for the lack of progress and updates here. 2022 was an awful year for me, I had to move countries, and I didn't really have time time to finish this PR.

I don't think that I'll be able to finish this anytime soon, so if anyone wants to pick this up, I'll be happy to share the existing code and findings. If not, that's fine too, I think I'll get back to it eventually, but I can't really say when.

[davidhewitt]

@daniil-konovalenko no problem, hope things settle down soon for you and family.

I have had this PR in the back of my mind, I think I'd like to create a pyo3-typed-collections library which has helpers like PyList<T> as an experiment. I'd pull this PR onto that library to see how it feels. Unfortunately I haven't gotten there yet 😄