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

Add context manager example to user guide #1476

Merged
merged 10 commits into from
Mar 13, 2021
Merged

Conversation

isosphere
Copy link
Contributor

A simple illustrative example on how to use context managers. I required this to use pymc3, which relies heavily on the context stack for modelling.

My example has a gap in that it calls __exit__ with blank arguments. I don't honestly know how to use it without them, or if I even need to call it at all. If anyone wants to add some detail there, I'd appreciate it!

A simple illustrative example on how to use context managers. I required this to use pymc3, which relies heavily on the context stack for modelling.
@isosphere
Copy link
Contributor Author

CI failure looks unrelated:


failures:

---- types::string::test::test_extract_char stdout ----
---- types::string::test::test_extract_char stderr ----
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: PyErr { type: <class 'ValueError'>, value: ValueError('expected a string of length 1'), traceback: None }', src/types/string.rs:239:73
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    types::string::test::test_extract_char

test result: FAILED. 236 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 4.32s

@davidhewitt
Copy link
Member

Thanks, will read later! Yes, the coverage failure is a regression in upstream Rust nightly that'll be fixed in good time ;)

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much for writing this. You're right that the __exit__ part probably needs some expansion - I've added a comment below.

guide/src/python_from_rust.md Outdated Show resolved Hide resolved
@isosphere
Copy link
Contributor Author

isosphere commented Mar 8, 2021

Thanks for your suggestion! I ran with it, and did a little eval with an undefined variable which will always throw an exception. Here's the output I get:

Welcome to 123 Main Street!
Sorry you had <class 'NameError'> trouble at 123 Main Street

Your use of py.None() there results in a moved value, so I passed it by reference (even though that branch of the match will never execute, but it should be helpful as an example of what to do).

However, I'm not certain that it's okay to pass py.None() by reference here.

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, just two final nits!


"#, "objects.py", "objects").unwrap();

let house = custom_manager.call1("House", ("123 Main Street",)).unwrap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be call_function1, because we actually just deprecated call1 on master.

@birkenfeld strictly speaking this isn't a function, so maybe call_attribute is better? Although I wonder if actually we should plan to remove module::call{_function} etc. completely and instead just ask users to do .getattr("...")?.call1("...")? etc? It's not much longer and doesn't suffer with naming ambiguity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you must mean call_method1, so I've changed it to that!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See e065f9b - call_function1 is very hot off the press :)

Copy link
Contributor Author

@isosphere isosphere Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, you updated the documentation in that commit. Ignore earlier message if you saw it! I'll make the change!

Changes made, though I'm having trouble using that code in a little test bed I made. My cargo.toml has

[dependencies]
pyo3 = {git = "https://github.com/PyO3/pyo3.git", features=["auto-initialize"]}

That change is in master, so I figure it should be active. Did a cargo clean for fun.

cargo run gives

   Compiling proc-macro2 v1.0.24
   Compiling unicode-xid v0.2.1
   Compiling syn v1.0.62
   Compiling proc-macro-hack v0.5.19
   Compiling winapi v0.3.9
   Compiling cfg-if v1.0.0
   Compiling libc v0.2.88
   Compiling unindent v0.1.7
   Compiling smallvec v1.6.1
   Compiling scopeguard v1.1.0
   Compiling pyo3 v0.13.2 (https://github.com/PyO3/pyo3.git#b75a8f6e)
   Compiling instant v0.1.9                                                                                                                                                                          
   Compiling lock_api v0.4.2                                                                                                                                                                         
   Compiling quote v1.0.9                                                                                                                                                                            
   Compiling paste-impl v0.1.18
   Compiling paste v0.1.18
   Compiling parking_lot_core v0.8.3
   Compiling parking_lot v0.11.1
   Compiling pyo3-macros-backend v0.13.2 (https://github.com/PyO3/pyo3.git#b75a8f6e)
   Compiling indoc-impl v0.3.6
   Compiling indoc v0.3.6
   Compiling pyo3-macros v0.13.2 (https://github.com/PyO3/pyo3.git#b75a8f6e)
   Compiling pyo3-context-example v0.1.0 (C:\Users\Matt\Documents\Development\Rust\pyo3-context-example)
error[E0599]: no method named `call_function0` found for reference `&PyAny` in the current scope
  --> src\main.rs:22:15
   |
22 |         house.call_function0("__enter__").unwrap();
   |               ^^^^^^^^^^^^^^ method not found in `&PyAny`

error[E0599]: no method named `call_function1` found for reference `&PyAny` in the current scope
  --> src\main.rs:30:23
   |
30 |                 house.call_function1("__exit__", (&none, &none, &none)).unwrap();
   |                       ^^^^^^^^^^^^^^ method not found in `&PyAny`

error[E0599]: no method named `call_function1` found for reference `&PyAny` in the current scope
  --> src\main.rs:33:23
   |
33 |                 house.call_function1(
   |                       ^^^^^^^^^^^^^^ method not found in `&PyAny`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0599`.
error: could not compile `pyo3-context-example`

To learn more, run the command again with --verbose.

I'm probably missing something?

guide/src/python_from_rust.md Show resolved Hide resolved
Comment on lines +264 to +265
// If the eval threw an exception we'll pass it through to the context manager.
// Otherwise, __exit__ is called with empty arguments (Python "None").
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

@kngwyu kngwyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple and illustrative example 👍🏼 , thank you!

else:
print(f"Thank you for visiting {self.address}, come again soon!")

"#, "objects.py", "objects").unwrap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit, but is house.py better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, changed!

guide/src/python_from_rust.md Outdated Show resolved Hide resolved
guide/src/python_from_rust.md Outdated Show resolved Hide resolved
@birkenfeld
Copy link
Member

As for call_function vs call_method vs call: you're right, while call_function is better than call, it's not really such a good name and call_attr would be better. Or even no special API.

In the end I wish we could implement some Fn trait for PyAny which would make this much easier :)

@davidhewitt
Copy link
Member

In the end I wish we could implement some Fn trait for PyAny which would make this much easier :)

I have been abstractly dreaming recently about a macro to solve all this, spread args and kwargs and also use the Vectorcall optimization. E.g.:

py_call!(module.attr(a, b, *args, **kwargs));

but that's probably a pipe dream 😄

isosphere and others added 2 commits March 13, 2021 14:38
Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
@davidhewitt
Copy link
Member

👍 Thanks very much for this example; sorry for the complications caused by our API changes.

@davidhewitt davidhewitt merged commit e24f29d into PyO3:master Mar 13, 2021
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.

4 participants