pyo3 icon indicating copy to clipboard operation
pyo3 copied to clipboard

feat: add type alias `InternString` for macro `intern!`

Open Stargateur opened this issue 1 year ago • 1 comments

Stargateur avatar Jul 20 '24 06:07 Stargateur

Thanks, I can kind of see why this is helpful though I also wonder why this is happening enough that this seems worth having in PyO3 itself rather than user code? It's relatively trivial for this one-liner to be written downstream by those who want it.

Well, trivial or not that one line every user will maybe add when pyo3 can do it. that the main purpose of lib, share common code. I see no con to this cause change the return type of intern! would be breaking change anyway. This also add clarity cause the return type of the macro was undocumented.

I need this cause the macro would create a string if I call it from multiple location, so I used function that return the string:

/// Return the interned string "foo".
fn foo(py: Python) -> InternString {
    intern!(py, "foo")
}

was more clear than:

/// Return the interned string "foo".
fn foo(py: Python) -> &pyo3::Bound<PyString> {
    intern!(py, "foo")
}

Stargateur avatar Jul 22 '24 02:07 Stargateur

Revisiting this one, I realise another downside (IMO) of the type spread is that not so &Bound<'py, PyString> come from interning. So I think the type alias risks creating confusion.

I think it's fine in user code if it helps you, but hold my opinion that this isn't clearly a good thing to add to the library so will choose to choose this PR.

Thank you for the suggestion and discussion, sorry it didn't get merged this time.

davidhewitt avatar Oct 21 '24 17:10 davidhewitt