[Sinkleberg]

For #310. NotFilter(*filters) keeps an object when the wrapped filter chain would drop it. It works with native filters and Python callbacks.

I used Codex to help with this change. On Windows I rebuilt the filter module against the released native core. The suite passed 453 tests and skipped the optional Shapely example.

[lonvia]

Thanks but I would prefer a solution where there is a NotFilter() which can be wrapped around a filter chain, so that inversion can be applied to any filter.

[Sinkleberg]

Thanks for the suggestion. I switched this to NotFilter(*filters) so it works with a whole chain.

[lonvia]

Why all this custom constructor handling? Why doesn't pybind11 take care of that in this case?

[Sinkleberg]

Construction still uses py::init. The extra hooks are there because HandlerChain borrows its filters, so NotFilter keeps their Python objects alive. If a filter points back to its wrapper, Python can't see that cycle through the C++ member. Without the hooks, both objects survived gc.collect() in the regression test. The holder check covers collection before construction finishes.

[lonvia]

Ah, you don't need to keep the input argument. HandlerChain already takes care to keep a reference of the handler. In fact, this code won't do what you expect. The __init__ function made its own copy of the list. Modifying it later has no affect on the handler.

Not to speak of the fact that doing something like this would anyway create an endless loop where a dangling referece is the least of your worries.

[Sinkleberg]

Agreed, the chain is fixed at construction. wrappers.append(...) just records a weak reference for the test. handler.owner = wrapper creates a reference cycle, but the callback only returns True, so it never calls the wrapper again.

My concern is the temporary in NotFilter(MyFilter()). HandlerChain stores a raw pointer for native handlers, and PythonHandler stores a non-owning pybind11::handle for Python callbacks. If we remove m_args, what keeps that temporary alive?

[lonvia]

Well, then that rather points to a potential bug in HandlerChain. That obviously shouldn't be silently worked around in a new filter implementation but be addressed.

We are at the end of AI-driven coding here. I'll need to look into the code and see what's going on with HandlerChain. Until then this PR needs to be stalled.

[lonvia]

Uh, I just realised that you are repo hoping with mass AI-generated PRs. Those I don't take anyway. So closing.