smaheshwar-pltr commented on PR #1783: URL: https://github.com/apache/iceberg-python/pull/1783#issuecomment-2746204012
Thanks for taking a look folks, and apologies for the delayed response. > are there any reasons we don't want to do this? This approach serves only to circumvent recursion. When it's said that "iteration is faster than recursion", I don't think it refers to just concretising the frames / call-stack in memory - I believe this aligns with @Fokko mentioning that performance might be worsened, which I agree with. If we *do* want to go with this PR's approach, then I think we should consider it for the other visitors in `visitors.py` (after benchmarking performance). I also wonder if (a) some decorator magic is possible for the conversion, because IMO these simple recursive visitors read nicer and feel less error-prone (we can rewrite to be tail-recursive and use [`tco`](https://macropy3.readthedocs.io/en/latest/tco.html#tail-call-optimization) but I think the rewrite introduces complexity similar to this PR), or if (b) there's some way to un-intrusively keep the current recursive approach. I see some recent activity / PRs on the original issue so happy to wait for that discussion to conclude. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
