Skip to content
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

IronPython 3 Compatibility and Update for xlrd & xlsxwriter #2422

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

trgiangv
Copy link

Description:

This pull request introduces:

  1. IronPython 3 Compatibility:
  • Simplify Compatible variable for handling IronPython 2/3, Cpython, netcore, netframework. trgiangv@cf51642
  • Ensured existing functionality remains stable across both versions.
  1. Updated xlrd and xlsxwriter Library:

    • Updated to the latest version.
    • Ironpython3 compatible
    • Fixed image import issues, ensuring proper embedding and display of images in Excel files.
  2. Fix build pyrevitLabs.pyRevit.Runtime only target IronPython 2.7

Additional Notes:

What challenges are we facing if we set IronPython3 as the default engine?

@jmcouffin jmcouffin added Enhancement Enhancement request [class->Improved #{number}: {title}] Python 3 Issues related to cpython engines [subsystem] IronPython Issues related to standalone IronPython installation [subsystem] Highlight Highlight these issues on Release notes dependencies Pull requests that update a dependency file labels Oct 18, 2024
@dosymep
Copy link
Member

dosymep commented Oct 18, 2024

I think it's worth splitting the pull request into several,

  • changing the pyRevit
  • changing dependencies

I think there may be problems with dependencies and changing them, after which there will be problems with compatibility with the pyRevit code.

Also, can you resolve the merge issues?

Update:

I checked the code superficially, there is use of libraries only here, perhaps there will not be so many problems with changing versions.

"""Read and Write Excel Files."""

Copy link
Contributor

@sanzoghenzo sanzoghenzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done @trgiangv!

Aside for the merge conflicts to resolve, I've pointed out a minor change.

Thank you for your work!

pyrevitlib/pyrevit/routes/server/base.py Outdated Show resolved Hide resolved
pyrevitlib/pyrevit/__init__.py Outdated Show resolved Hide resolved
pyrevitlib/pyrevit/compat.py Outdated Show resolved Hide resolved
pyrevitlib/pyrevit/forms/__init__.py Outdated Show resolved Hide resolved
@trgiangv
Copy link
Author

trgiangv commented Oct 19, 2024

I think it's worth splitting the pull request into several,

  • changing the pyRevit
  • changing dependencies

I think there may be problems with dependencies and changing them, after which there will be problems with compatibility with the pyRevit code.

Also, can you resolve the merge issues?

Update:

I checked the code superficially, there is use of libraries only here, perhaps there will not be so many problems with changing versions.

"""Read and Write Excel Files."""

Hi @dosymep

xlrd: just fix the IronPython3 compatible
XlsxWriter: I've got the problem with writing images to the Excel file, update to the latest version solve that
pyrevitLabs.pyRevit.Runtime: Currently, I have no idea to fix this, I can only identify the problem so that someone else may address it. However, rebuilding locally with IronPython3 resolved the problem for me.

Use DOTNET_RUNTIME_ID instead of net_folder
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Enhancement Enhancement request [class->Improved #{number}: {title}] Highlight Highlight these issues on Release notes IronPython Issues related to standalone IronPython installation [subsystem] Python 3 Issues related to cpython engines [subsystem]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants