Move macros into separate feature. #897

m-ou-se · 2020-05-04T15:25:45Z

This makes it possible to opt-out of all the (proc) macros, which reduces the amount of dependencies that need to be downloaded and built.

Together with #895, #896 and #899, this reduces the dependency tree from:

pyo3 v0.9.2
├── indoc v0.3.5
│   ├── indoc-impl v0.3.5
│   │   ├── proc-macro-hack v0.5.15
│   │   ├── proc-macro2 v1.0.10
│   │   │   └── unicode-xid v0.2.0
│   │   ├── quote v1.0.3
│   │   │   └── proc-macro2 v1.0.10 (*)
│   │   ├── syn v1.0.17
│   │   │   ├── proc-macro2 v1.0.10 (*)
│   │   │   ├── quote v1.0.3 (*)
│   │   │   └── unicode-xid v0.2.0
│   │   └── unindent v0.1.5
│   └── proc-macro-hack v0.5.15
├── inventory v0.1.6
│   ├── ctor v0.1.14
│   │   ├── quote v1.0.3 (*)
│   │   └── syn v1.0.17 (*)
│   ├── ghost v0.1.1
│   │   ├── proc-macro2 v1.0.10 (*)
│   │   ├── quote v1.0.3 (*)
│   │   └── syn v1.0.17 (*)
│   └── inventory-impl v0.1.6
│       ├── proc-macro2 v1.0.10 (*)
│       ├── quote v1.0.3 (*)
│       └── syn v1.0.17 (*)
├── libc v0.2.69
├── num-traits v0.2.11
│   [build-dependencies]
│   └── autocfg v1.0.0
├── parking_lot v0.10.2
│   ├── lock_api v0.3.4
│   │   └── scopeguard v1.1.0
│   └── parking_lot_core v0.7.2
│       ├── cfg-if v0.1.10
│       ├── libc v0.2.69
│       └── smallvec v1.4.0
├── paste v0.1.11
│   ├── paste-impl v0.1.11
│   │   ├── proc-macro-hack v0.5.15
│   │   ├── proc-macro2 v1.0.10 (*)
│   │   ├── quote v1.0.3 (*)
│   │   └── syn v1.0.17 (*)
│   └── proc-macro-hack v0.5.15
├── pyo3cls v0.9.2
│   ├── pyo3-derive-backend v0.9.2
│   │   ├── proc-macro2 v1.0.10 (*)
│   │   ├── quote v1.0.3 (*)
│   │   └── syn v1.0.17 (*)
│   ├── quote v1.0.3 (*)
│   └── syn v1.0.17 (*)
└── unindent v0.1.5
[build-dependencies]
├── regex v1.3.7
│   └── regex-syntax v0.6.17
├── serde v1.0.106
│   └── serde_derive v1.0.106
│       ├── proc-macro2 v1.0.10 (*)
│       ├── quote v1.0.3 (*)
│       └── syn v1.0.17 (*)
├── serde_json v1.0.51
│   ├── itoa v0.4.5
│   ├── ryu v1.0.3
│   └── serde v1.0.106 (*)
└── version_check v0.9.1

to:

pyo3 v0.9.2
└── libc v0.2.69
[build-dependencies]
└── version_check v0.9.1

with --no-default-features.

davidhewitt · 2020-05-04T18:48:45Z

Hmmm this is an interesting idea! In principle I am keen to give users an option to have a smaller compile for sure.

I wonder - once the macros are removed then a lot of other code becomes irrelevant I think - e.g. PyCell, and the entirety of src/class. Rather than just drawing the line at macros, should we feature-gate just a little more code? This would get us a long way towards satisfying the recent suggestion in #5 to add a feature to subset pyo3 just to stable code.

m-ou-se · 2020-05-04T18:56:35Z

Yeah, with feature = "macros" a lot of the code is unused (or should be), but I didn't manage to find a nice line to cut things off at. I tried putting PyClass behind the feature = "macros" gate, but that broke a lot of other things. Also gating the things it broke, broke even more things, and so on. So in the end I gave up and just took the easy way out with a dummy implementation for PyMethodsImpl in the not(feature = "macros") case, and leaving all the PyClass stuff in place.

My main goal was to avoid pulling in unnecessary dependencies, so I'm not too worried about parts of the pyo3 crate that stay unused.

It would definitely be nice to split this better (maybe even separate crates?), but that's probably a lot of work.

davidhewitt · 2020-05-04T19:04:46Z

👍 maybe I will have a quick play with this later to see if I can find something which feels good.

I'm in split opinion about separate crates. It helps with some big projects but it does separate the documentation out into multiple places, which can make it a little bit harder for users to find the docs they need...

ijl · 2020-05-04T22:16:17Z

@m-ou-se how much of pyo3 outside of the FFI bindings does your use case exercise if it doesn't use pymodule, pyfunction, etc.? Would moving the FFI bindings into a pyo3ffi crate be useful?

kngwyu

Thank you!

It's a bit surprising for me that we can separate features by such a simple trick.

kngwyu · 2020-05-05T04:55:29Z

src/class/methods.rs

@@ -142,3 +144,11 @@ pub trait PyMethodsImpl {
            .collect()
    }
 }
+
+#[doc(hidden)]
+#[cfg(not(feature = "macros"))]


kngwyu · 2020-05-05T05:00:34Z

This would get us a long way towards satisfying the recent suggestion in #5 to add a feature to subset pyo3 just to stable code.

You mean we should separate PyClass? If so, please wait until my next trial to remove specialization fails.

Would moving the FFI bindings into a pyo3ffi crate be useful?

@ijl
It's OK to separate FFI parts if you need them. For orjson?
My only concern is how the build script should be.

m-ou-se · 2020-05-05T06:15:34Z

@ijl

@m-ou-se how much of pyo3 outside of the FFI bindings does your use case exercise if it doesn't use pymodule, pyfunction, etc.? Would moving the FFI bindings into a pyo3ffi crate be useful?

The inline-python crate requires PyDict and ToPyObject and some other stuff, but none of the macros.

Another crate I'm about to publish converts objects to and frorm PyO3's PyObject using serde. It uses PyDict, PyList, PyTuple and more, but again no macros.

All the macros like #[pymodule] are mostly used when using Rust from Python. But when using Python from Rust, they're often not needed (in simple cases).

kngwyu · 2020-05-05T13:36:25Z

src/lib.rs

@@ -280,6 +278,7 @@ macro_rules! wrap_pymodule {
 /// If you need to handle failures, please use [Python::run] directly.
 ///
 #[macro_export]
+#[cfg(feature = "macros")]


I'm sorry for my lazy review but could you please add this definition?

#[macro_export] #[cfg(not(feature = "macros"))] macro_rules! py_run { ($py:expr, $($val:ident)+, $code:expr) => {{ pyo3::py_run_impl!($py, $($val)+, &pyo3::unindent::unindent($code)) }}; }

unindent is a really small dependency (see its Cargo.toml) and I think it's OK.

then py_run would behave differently depending on the feature? that seems confusing...

That would mean that code like

let code = "asdf"; py_run!(py, a b, code);

would compile fine with feature = "macros" disabled, but will no longer compile when it gets enabled.

then py_run would behave differently depending on the feature?

No, it behaves the same.
But

with feature="macros" and

string literal is passed like py_run!(py, a, "print(a)")

it works faster thanks to indoc.

ijl · 2020-05-05T15:37:46Z

I don't have a need to separate the FFI right now. It is simple to do, though. The build.rs in each should be duplicated, but this is rarely touched and I think pretty mild for how difficult some build processes can be, so not that bad. So if it helps #904 or something else it's simple to do (I have a branch if needed).

kngwyu · 2020-05-09T04:20:40Z

Could you please rebase on the master?
Github doesn't allow maintainers to edit an organization branch,

It's enabled by default to avoid breakage, but this allows compiling pyo3 with a lot less dependencies in case the macros are not needed.

kngwyu · 2020-05-11T02:01:42Z

Thanks!

m-ou-se force-pushed the macros-feature branch from ada41c1 to 780f5fd Compare May 4, 2020 15:52

kngwyu approved these changes May 5, 2020

View reviewed changes

kngwyu closed this May 5, 2020

kngwyu reopened this May 5, 2020

kngwyu reviewed May 5, 2020

View reviewed changes

kngwyu mentioned this pull request May 9, 2020

Use PyMethodsImpl instead of *ProtocolImpl::methods #917

Merged

Move macros into separate feature.

dab8c2e

It's enabled by default to avoid breakage, but this allows compiling pyo3 with a lot less dependencies in case the macros are not needed.

m-ou-se force-pushed the macros-feature branch from 780f5fd to dab8c2e Compare May 9, 2020 09:27

kngwyu merged commit da22eec into PyO3:master May 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move macros into separate feature. #897

Move macros into separate feature. #897

m-ou-se commented May 4, 2020 •

edited

Loading

davidhewitt commented May 4, 2020 •

edited

Loading

m-ou-se commented May 4, 2020

davidhewitt commented May 4, 2020

ijl commented May 4, 2020

kngwyu left a comment

kngwyu May 5, 2020

kngwyu commented May 5, 2020 •

edited

Loading

m-ou-se commented May 5, 2020 •

edited

Loading

kngwyu May 5, 2020

birkenfeld May 5, 2020

m-ou-se May 5, 2020 •

edited

Loading

kngwyu May 5, 2020 •

edited

Loading

ijl commented May 5, 2020 •

edited

Loading

kngwyu commented May 9, 2020

kngwyu commented May 11, 2020

Move macros into separate feature. #897

Move macros into separate feature. #897

Conversation

m-ou-se commented May 4, 2020 • edited Loading

davidhewitt commented May 4, 2020 • edited Loading

m-ou-se commented May 4, 2020

davidhewitt commented May 4, 2020

ijl commented May 4, 2020

kngwyu left a comment

Choose a reason for hiding this comment

kngwyu May 5, 2020

Choose a reason for hiding this comment

kngwyu commented May 5, 2020 • edited Loading

m-ou-se commented May 5, 2020 • edited Loading

kngwyu May 5, 2020

Choose a reason for hiding this comment

birkenfeld May 5, 2020

Choose a reason for hiding this comment

m-ou-se May 5, 2020 • edited Loading

Choose a reason for hiding this comment

kngwyu May 5, 2020 • edited Loading

Choose a reason for hiding this comment

ijl commented May 5, 2020 • edited Loading

kngwyu commented May 9, 2020

kngwyu commented May 11, 2020

m-ou-se commented May 4, 2020 •

edited

Loading

davidhewitt commented May 4, 2020 •

edited

Loading

kngwyu commented May 5, 2020 •

edited

Loading

m-ou-se commented May 5, 2020 •

edited

Loading

m-ou-se May 5, 2020 •

edited

Loading

kngwyu May 5, 2020 •

edited

Loading

ijl commented May 5, 2020 •

edited

Loading