-
Notifications
You must be signed in to change notification settings - Fork 56
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
Variable type annotations for homogeneous tuples #1575
Conversation
Hello again! Thank you for this new pull request 🤩. Please begin by requesting your checklist using the command |
e362daf
to
0e5f5e8
Compare
0e5f5e8
to
95e717c
Compare
/bot run linux |
/bot run pylint pyccel_lint docs spelling |
/bot run pylint docs |
Here is your checklist. Please tick items off when you have completed them or determined that they are not necessary for this pull request:
|
/bot run coverage pylint pyccel_lint docs spelling |
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.
There seems to be lines in this PR which aren't tested. Please take a look at my comments and add tests which cover the new code.
If this is modified code which cannot be easily tested in this PR please open an issue to request that this code be either removed or tested. Once you have done that please leave a message on the relevant conversation beginning with the line /bot accept
and referencing the issue.
Similarly if the new code cannot be tested for some reason, please leave a comment beginning with the line /bot accept
on the relevant conversation explaining why the code can't be tested.
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.
Good job ! Your PR is using all the code it added/changed.
Hey @pyccel/pyccel-dev ! @EmilyBourne has just created this great new pull request! Check it out and let me know what you think! |
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.
Everything looks good to me; the code changes seem understandable, and the documentation is clear.
Hey @yguclu, @EmilyBourne, this PR is looking pretty good. @EmilyBourne and @Farouk-Echaref think it is ready to merge. Could you add your expertise to confirm that this follows all the coding conventions and fits in Pyccel's future plans? Thanks 😄 |
/bot run pr_tests |
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.
Good job ! Your PR is using all the code it added/changed.
Handle the indexing of a tuple class for type annotations. For now only homogeneous tuples are handled. Inhomogeneous tuples will be simple to add once
InhomogeneousTupleVariable
is deprecated.Fix handling of homogeneity in
PythonTuple
. Fixes #1182Commit Summary
PythonTuple
PythonMin
andPythonMax
LiteralEllipsis
class to help describe homogeneous tuple type annotationsNumpyArray
_visit_X
functions to be added for built-in classes.pyccel_to_sympy
to allow the use ofa*(3-1)
syntax wherea
is a tuple. Previously this was precluded as3-1
is not an integer