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

release: 0.16.0 #2174

Merged
merged 1 commit into from Feb 27, 2022
Merged

release: 0.16.0 #2174

merged 1 commit into from Feb 27, 2022

Conversation

davidhewitt
Copy link
Member

@davidhewitt davidhewitt commented Feb 17, 2022

Release branch for 0.16.0 release.

There's a couple PRs which I'd like to let merge (and rebase on) before this goes live:

Hopefully we can have this ready to go live around the weekend at the end of the month (26th./ 27th Feb).

@davidhewitt
Copy link
Member Author

Rebased. I think this is on track to go out at the weekend.

It would be nice if #2175 and #2183 are able to be merged before the release goes out.

I'll hopefully merge #2143 this morning as soon as CI passes.

@davidhewitt
Copy link
Member Author

Rebased one final time. I'll put this live tomorrow evening.

I might rebase to include #2187 if that merges, however I also think it's fine if that lands as part of whatever release follows 0.16.

@birkenfeld
Copy link
Member

birkenfeld commented Feb 26, 2022

I just switched a codebase to main and had a small snag: I was defining pymodules in Rust-submodules and got:

error[E0425]: cannot find value `__PYO3_PYMODULE_DEF_REMOTE` in this scope
  --> src/lib.rs:15:19
   |
15 |     m.add_wrapped(wrap_pymodule!(remote))
   |                   ^^^^^^^^^^^^^^^^^^^^^^ not found in this scope
   |
note: static `crate::remote::__PYO3_PYMODULE_DEF_REMOTE` exists but is inaccessible
  --> src/remote.rs:12:1
   |
12 | #[pymodule]
   | ^^^^^^^^^^^ not accessible
   = note: this error originates in the macro `wrap_pymodule` (in Nightly builds, run with -Z macro-backtrace for more info)

The problem being that the #[pymodule] fn was not declared pub. This seems to have been unproblematic in 0.15, but 0.16 transfers the function visibility to the module-def (which is a good thing!)

Is it worth writing a migration entry for this?

@mejrs
Copy link
Member

mejrs commented Feb 26, 2022

Sorry for being a bit late but I thought about examples for the gc and buffer protocols and noticed a couple things that need fixing before 0.16

  • pyproto feature is not documented in lib.rs
  • The pyproto feature must be activated to implement __traverse__ in #[pymethods]. The following does not work:
[dependencies]
pyo3 = {path = "../pyo3", default-features = false, features = ["macros"]}
use pyo3::prelude::*;
use pyo3::PyTraverseError;
use pyo3::PyVisit;

#[pyclass]
struct Thing;

#[pymethods]
impl Thing {
    fn __traverse__(&self, _visit: PyVisit) -> Result<(), PyTraverseError> {
        Ok(())
    }

    fn __clear__(&mut self) {
    }
}

results in:

error[E0432]: unresolved import `pyo3::PyTraverseError`
 --> src\lib.rs:2:5
  |
2 | use pyo3::PyTraverseError;
  |     ^^^^^^^^^^^^^^^^^^^^^ no `PyTraverseError` in the root

error[E0432]: unresolved import `pyo3::PyVisit`
 --> src\lib.rs:3:5
  |
3 | use pyo3::PyVisit;
  |     ^^^^^^^^^^^^^ no `PyVisit` in the root

error[E0433]: failed to resolve: could not find `gc` in `class`
 --> src\lib.rs:8:1
  |
8 | #[pymethods]
  | ^^^^^^^^^^^^ could not find `gc` in `class`
  |
  = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)

because the pyproto feature must be enabled for those imports.

  • Are/should be the __getbuffer__/__releasebuffer__ method be unsafe to implement, rather than just unsafe to call?

@davidhewitt
Copy link
Member Author

Ah, thanks both. I've got a moment of calm this evening so I'll push a fix for these points shortly.

Are/should be the __getbuffer__/__releasebuffer__ method be unsafe to implement, rather than just unsafe to call?

@mejrs can you elaborate on this point a little? We require the user to use unsafe keyword in the implementation, are you thinking we should do something different?

@mejrs
Copy link
Member

mejrs commented Feb 26, 2022

A trait can be unsafe to implement and unsafe to call. For example stds Allocator. dealloc is unsafe because the caller has to provide a valid pointer and allocate is unsafe on behalf of the implementer, because the caller has to trust the implementer that the returned pointers are valid.

So my question is, is the buffer protocol also like that? Fwiw I am fine with "the user has to type an unsafe keyword somewhere, that is enough and nothing needs to change".

But for example we can have unsafe pymethod blocks...

#[pymethods]
unsafe impl Foo {}

...if the macro removes the unsafe keyword from the tokenstream

@mejrs
Copy link
Member

mejrs commented Feb 26, 2022

To clarify I am not saying that this needs to change, just that we might want to make a decision on it.

@davidhewitt
Copy link
Member Author

davidhewitt commented Feb 26, 2022

👍 I think making a decision on any change there is probably too much just before the release. We can always reconsider and make tweaks for a future release.

unsafe pymethod blocks

that's a really interesting idea. I think that while multiple-pymethods is dependent on inventory which isn't universally supported, I'd rather not go down that road. In the future, it could definitely make sense.

(EDIT fat fingers sent early!)

@davidhewitt
Copy link
Member Author

Rebased to pick up #2192, this is now the version I expect to put live this evening.

@davidhewitt
Copy link
Member Author

Release is live 🎉

@davidhewitt davidhewitt merged commit 537c711 into main Feb 27, 2022
@davidhewitt davidhewitt deleted the release-0.16 branch October 20, 2022 06:48
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

Successfully merging this pull request may close these issues.

None yet

3 participants