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

conversions and #[pyo3(get, set)] for atomic integers #4668

Open
davidhewitt opened this issue Oct 30, 2024 · 4 comments
Open

conversions and #[pyo3(get, set)] for atomic integers #4668

davidhewitt opened this issue Oct 30, 2024 · 4 comments

Comments

@davidhewitt
Copy link
Member

In #4566 we ran into the fact that atomic integers don't currently work as #[pyclass] fields.

We should decide what conversions we want on these types. We can probably implement:

  • IntoPyObject and FromPyObject for owned values, because there is no atomic contention in these cases.
  • Should we implement IntoPyObject for &AtomicX ? It's not clear to me that we can always just use Relaxed ordering there.
  • We should probably specialize the way that #[pyclass(get, set)] work so that Relaxed ordering is used to write #[pyclass] fields. This is probably ok for those uses?
@alex
Copy link
Contributor

alex commented Oct 30, 2024 via email

@ngoldbaum
Copy link
Contributor

ngoldbaum commented Oct 30, 2024

Instead of modeling it on the python side using a PyInt we could use a type that represents an explictly atomic fixed-size int.

The ft_utils package from meta defines some python types that wrap atomics (e.g. AtomicInt64). There might be some room for inspiration there.

@davidhewitt
Copy link
Member Author

I'm a bit concerned that allowing atomics to be converted to Python ints transparently would lead to unexpected behavior with +=

Ah, that's a very good point. I think that's reason enough to not support get / set for now (at least not without some kind of upstream support in the interpreter.

Instead of modeling it on the python side using a PyInt we could use a type that represents an explictly atomic fixed-size int.

The ft_utils package from meta defines some python types that wrap atomics (e.g. AtomicInt64). There might be some room for inspiration there.

I'm open to the possibility that we'd need something like this, however quite reluctant. So far we've resisted adding framework-level types in PyO3 (PanicException being the only one); we haven't really found a nice way to expose them for users to import / interact with etc.

@mejrs
Copy link
Member

mejrs commented Oct 30, 2024

I think we shouldn't support it for now. We can revisit this in the future when the demand arises.

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

No branches or pull requests

4 participants