-
-
Notifications
You must be signed in to change notification settings - Fork 393
PY3 compatibility for itertools #2811
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary:
Fixes itertools compatibility for IronPython 3 by replacing the deprecated izip_longest
import. The change conditionally imports izip_longest
from itertools for PY2 and zip_longest as izip_longest
for PY3, resolving the AttributeError that prevented print_table()
from working in IPY3.
Review Summary:
This is a focused bug fix that correctly addresses the itertools compatibility issue reported in #2810. The implementation follows established patterns in the pyrevit.compat module and maintains backward compatibility. The change is minimal, targeted, and directly resolves the AttributeError without introducing complexity or breaking existing functionality.
Follow-up suggestions:
@devloai add unit tests for the print_table() function to ensure compatibility across both IronPython 2 and 3
https://github.com/IronLanguages/ironpython3/wiki/Upgrading-from-IronPython2#redirecting-output This might have sth to do with that weird print statement? |
AI to the rescue: Markdown not rendered in IPy 3.4 → Output buffering / stream behavior change (binary vs text writer) I can confirm that this inside the output ini works:
and i assume that changing dev>modules>pyReviLabs.ironpython3>Src>DLR>Src>Microsoft.Scripting>Runtime>ShareIO.cs But what are the downsides of either solution? |
you could add and test adding if PY3:
sys.stdout.flush() to the print_md function. thoughts @sanzoghenzo @dosymep |
while you're at it, can you drop the I don't see any benefit of using that support variable.
I would go with this
since one of the goals I had in mind when we talked about isolating the plugin to avoid DLL hell was to drop our custom libraries and leverage the upstream ones via nuget (just to avoid having to remember what we patched when we need to update it) and use ilrepack or something like that. But I'm not the guru on this matter 😅 |
On a more general note, I believe we should start to rely more and more on six. It's already in the |
So I tried it with six, but i guess that site-package is loaded later? Which is a shame, would have made it more compact. |
site-package should be added to the path by the ironpython executor, so six should be there... What did you try? Does |
Yeah, i tried both: During startup i get the if i add it later, and hit reload in the pyrevit tab, it works |
Oh, I didn't think about pyrevitloader, it also loads pyrevit library. But I believe we can postpone it to another PR 😉 |
So should i drop this PR and wait for the pyrevitloader→pyrevitlib fix? |
Sorry for not being clear, the PR is OK as it is. I was referring to introducing six, it should be done extensively on the entire codebase, so I believe it is out of scope of this PR and can be postponed. Unless you want to fix this right away! |
📦 New work-in-progress (wip) builds are available for 5.2.0.25272+2017-wip |
📦 New work-in-progress (wip) builds are available for 5.2.0.25272+2149-wip |
📦 New work-in-progress (wip) builds are available for 5.2.0.25274+1734-wip |
📦 New work-in-progress (wip) builds are available for 5.2.0.25277+1425-wip |
📦 New work-in-progress (wip) builds are available for 5.2.0.25277+1427-wip |
Description
Fixes itertools compatibility
Checklist
Before submitting your pull request, ensure the following requirements are met:
Related Issues
If applicable, link the issues resolved by this pull request:
Additional Notes
The table won't show up in IPY3 unless i put a empty print statement before end of the print_table function - no idea why. IPY2 works fine. Like so:
Thank you for contributing to pyRevit! 🎉