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

Remove ast.unparse and removeprefix usage for better compatibility with older python versions #87

Closed
wants to merge 2 commits into from

Conversation

treo
Copy link
Contributor

@treo treo commented Jan 19, 2022

Description

We have identified that those two methods (str.removeprefix and ast.unparse) stop us from supporting python versions < 3.9. As version 3.7 is still officially supported for at least another 17 months (according to https://endoflife.date/python) we want to support that version too.

Pull Request Type

  • Xpipes Core (Jupyterlab Related changes)
  • Xpipes Canvas (Custom RD Related changes)
  • Xpipes Component Library
  • 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

  1. Manual test for same output as before

    1. Start jupyter lab
    2. get output at
      http://localhost:8888/xpipes/components/
    3. Check it is still the same as previously

Tested on?

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

@treo treo requested review from AdrySky and MFA-X-AI January 19, 2022 10:43
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.

Testing on python 3.7:

As per our discussion, it appears that there's 2 main breaking causes of the current version of Xpipes.

  1. My convention of using print(f"{foo=}) which is introduced in python 3.8 bpo-36817. This means we can't have this way of printing at all, to which I removed and I got hit next by
  2. Old ast way of parsing things, shown by
[E 2022-01-20 00:27:24.561 ServerApp] Uncaught exception GET /xpipes/components/?1642613244660 (127.0.0.1)
    HTTPServerRequest(protocol='http', host='localhost:8888', method='GET', uri='/xpipes/components/?1642613244660', version='HTTP/1.1', remote_ip='127.0.0.1')
    Traceback (most recent call last):
      File "/home/fahreza-ubuntu/Github/xpipes-oldpy/xpipes/venv/lib/python3.7/site-packages/tornado/web.py", line 1702, in _execute
        result = method(*self.path_args, **self.path_kwargs)
      File "/home/fahreza-ubuntu/Github/xpipes-oldpy/xpipes/venv/lib/python3.7/site-packages/tornado/web.py", line 3173, in wrapper
        return method(self, *args, **kwargs)
      File "/home/fahreza-ubuntu/Github/xpipes-oldpy/xpipes/xpipes/handlers/components.py", line 120, in get
        components.extend(chain.from_iterable(self.extract_components(f, directory, python_path) for f in python_files))
      File "/home/fahreza-ubuntu/Github/xpipes-oldpy/xpipes/xpipes/handlers/components.py", line 120, in <genexpr>
        components.extend(chain.from_iterable(self.extract_components(f, directory, python_path) for f in python_files))
      File "/home/fahreza-ubuntu/Github/xpipes-oldpy/xpipes/xpipes/handlers/components.py", line 150, in extract_components
        for node in parse_tree.body if is_xai_component(node)]
      File "/home/fahreza-ubuntu/Github/xpipes-oldpy/xpipes/xpipes/handlers/components.py", line 150, in <listcomp>
        for node in parse_tree.body if is_xai_component(node)]
      File "/home/fahreza-ubuntu/Github/xpipes-oldpy/xpipes/xpipes/handlers/components.py", line 171, in extract_component
        for v in node.body if is_arg(v)
      File "/home/fahreza-ubuntu/Github/xpipes-oldpy/xpipes/xpipes/handlers/components.py", line 171, in <listcomp>
        for v in node.body if is_arg(v)
      File "/home/fahreza-ubuntu/Github/xpipes-oldpy/xpipes/xpipes/handlers/components.py", line 74, in read_orig_code
        line_from = node.lineno - 1
    AttributeError: 'Index' object has no attribute 'lineno'

Looks like it'll take some time to age the code so it's completely compatible with 3.7. Gonna try 3.8 next.

@MFA-X-AI
Copy link
Member

Testing with py38:

it appears to follow the same error with

AttributeError: 'Index' object has no attribute 'lineno'

After more investigation, it appears that the change has only been introduced in py39 as per this issue in another repo.
Bright side is if this can be updated, py37 should be compatible as well (To which I guess I'll have to update my print statements...).

@MFA-X-AI
Copy link
Member

As #161 has enabled the older python compatibility, I'm going to close this PR. Thanks for helping out!

@MFA-X-AI MFA-X-AI closed this Jun 24, 2022
@MFA-X-AI MFA-X-AI deleted the pd/python_version_compatibility branch August 5, 2022 03:17
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