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

⚡️Compatibility with python 3.8 and above versions #161

Merged
merged 12 commits into from
Jun 7, 2022

Conversation

mansouralawi
Copy link
Member

Description

We have identified that those two methods (str.removeprefix and ast.unparse) stop us from supporting python versions < 3.9. the current changes add support for python versions > 3.8.

AST parsing differences between different Python versions:

What is preventing Xircuits from running on Python 3.7 is the missing of end_lineno and end_col_offset from the AST parse.

References

this PR is based on an older PR #87

Pull Request Type

  • Xircuits Core (Jupyterlab Related changes)
  • Xircuits Canvas (Custom RD Related changes)
  • Xircuits Component Library
  • Testing Automation
  • Documentation
  • Others (Please Specify)

Type of Change

  • Refactoring
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Tests

  • Manual test for same Xircuits behaviour and features as before:

    1. Clone Xircuits
    2. Build Xircuits with python 3.8
    3. Run Xircuits
    4. Check if the components library (side bar) exist,
    5. Run a workflow
    6. Repeat with python versions 3.9 , 3.10 & 3.11

Tested on?

  • Windows
  • Linux Ubuntu
  • Centos
  • Mac
  • Others (State here -> xxx )

Notes

Add if any.

@mansouralawi mansouralawi requested review from MFA-X-AI and AdrySky June 1, 2022 04:26
@mansouralawi mansouralawi self-assigned this Jun 1, 2022
Copy link
Member

@MFA-X-AI MFA-X-AI left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
Having more python version reports are definitely important. Glad to know 3.10+ is also compatible with the current build.
During testing, I think there's still a print statement.

Name(id='any', ctx=Load())
Name(id='any', ctx=Load())

I've also noticed that we may need to take the

Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'getOptions')

bug in a higher priority. It triggered when I was creating the wheel for 3.8, then trying the component drop feature.

drop bug 2

It is not related to this PR I believe, as it also triggers when building without a package-lock.json. As per @AdrySky's comment, it may be that one of our jupyterlab packages are no longer compatible with RD. Will need further investigation before merging.

@mansouralawi
Copy link
Member Author

Thank you for your review.

I have removed the print statement.

Noted on the build issue, I will be waiting for an update on that to re-test this PR build again.

Copy link
Member

@MFA-X-AI MFA-X-AI left a comment

Choose a reason for hiding this comment

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

with bugfix #162, I can verify that things work out nicely.
Built the wheel using 3.8, installed the wheel.
Components are correctly rendered and I can run simple components in the template library. Great work!

@MFA-X-AI MFA-X-AI merged commit c288595 into master Jun 7, 2022
@MFA-X-AI MFA-X-AI deleted the mansour/python_version_compatibility branch June 7, 2022 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants