implement PyModuleMethods

[davidhewitt]

Split from #3606

[codspeed-hq]

CodSpeed Performance Report

Merging #3703 will not alter performance

_{Comparing davidhewitt:module2 (e852a4b) with main (6776b90)}

Summary

✅ 78 untouched benchmarks

[davidhewitt]

I might be able to drive up coverage by adding more tests & also following the same strategy of no-pool testing as per #3681 (comment)

[davidhewitt]

Well, I started down the road of adding the new module constructors, and deprecating the existing ones.

It creates a lot of documentation churn to deal with the deprecation warnings, which is pretty painful to deal with.

In the spirit of #3681 (comment), I'm now wondering if we should slow things down even more and start purely by introducing the _bound variants of the APIs without deprecating anything in PyO3 0.21.

Once we have a complete implementation released in 0.21 we can start thinking for what the best way to start encouraging users to migrate should be. This also gives more time for rust-numpy and other downstream crates to catch up.

[davidhewitt]

If we agree that not deprecating anything for now might be better, I'll also open a PR to undo the deprecations already merged.

[alex]

FWIW, from a user perspective, I'd find the warnings valuable. Basically every use of the old APIs is a performance opportunity, so having those clearly marked is helpful.

[davidhewitt]

I do agree with that too, and adding these warnings was helping me find all the locations internally in PyO3 where these are used.

I suppose we are happy with documentation being updated to encourage users to use the new APIs? It's more churn in our code here, but maybe we just accept that it's going to be period of a lot of churn in PyO3 and we just have to look forward to the "great reckoning release".

That said, introducing and deprecating the module constructors here produced a lot of busywork so I definitely don't want to do the deprecations as part of this diff.

[adamreichold]

Sorry for distributing the discussion over multiple places. (Maybe we should use #3882?) See also #3681 (comment) with the TL;DR that I think shipping without deprecations by default would be nicer. Downstream projects who want to commit to a complete migration can disable the pool feature.

[davidhewitt]

Thanks as always for the review, pushed an updated form of the commit here. I'll still defer adding the constructors / feature-toggled deprecations in a follow up PR.

Making these changes I also wondered whether we want to split the part which is for building new modules out into a PyModuleBuilder at some point in the future. The current API allows to import and modify builtins, for example. I think that's an unrelated complication which might make more sense to consider along with #3294 in a future simpler PyO3.