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

This should resolve test_subscript and test_declare_matrix test in python3.9 #64 #67

Closed
wants to merge 7 commits into from

Conversation

amwi04
Copy link

@amwi04 amwi04 commented Nov 22, 2020

@prabhuramachandran ,As I am new to this can you check this out.
As new changes in ast in pyhton 3.9.

#64

After this change

-- Docs: https://docs.pytest.org/en/stable/warnings.html
=============================================================== short test summary info ================================================================
FAILED compyle/tests/test_translator.py::test_annotated_function - AttributeError: 'Name' object has no attribute 'value'
FAILED compyle/tests/test_translator.py::test_cuda_conversion - AttributeError: 'Name' object has no attribute 'value'
FAILED compyle/tests/test_translator.py::test_opencl_conversion - AttributeError: 'Name' object has no attribute 'value'
FAILED compyle/tests/test_translator.py::test_cuda_local_conversion - AttributeError: 'Name' object has no attribute 'value'
=============================================== 4 failed, 201 passed, 91 skipped, 117 warnings in 7.14s ================================================

@pep8speaks
Copy link

pep8speaks commented Nov 22, 2020

Hello @amodwani! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 706:5: E129 visually indented line with same indent as next logical line

Comment last updated at 2020-11-23 12:43:49 UTC

@codecov
Copy link

codecov bot commented Nov 22, 2020

Codecov Report

Merging #67 (6f3788f) into master (4469a9a) will decrease coverage by 0.03%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #67      +/-   ##
==========================================
- Coverage   69.84%   69.81%   -0.04%     
==========================================
  Files          20       20              
  Lines        4063     4065       +2     
  Branches      779      780       +1     
==========================================
  Hits         2838     2838              
- Misses       1020     1021       +1     
- Partials      205      206       +1     
Impacted Files Coverage Δ
compyle/translator.py 92.43% <33.33%> (-0.36%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc3f1a6...6f3788f. Read the comment docs.

Copy link
Contributor

@prabhuramachandran prabhuramachandran left a comment

Choose a reason for hiding this comment

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

I think a simpler fix is possible and will send one in a separate PR along with regression testing for 3.9.

return '%s[%s]' % (
self.visit(node.value), self.visit(node.slice.value)
)
if (isinstance(node.slice, ast.Constant) or
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be compressed into:

if isinstance(node.slice, (ast.Constant, ast.Name, ast.BinOp, ast.Subscript)):
  ...

@prabhuramachandran
Copy link
Contributor

Thank you for this, closing since this is superseded by #68 .

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.

None yet

3 participants