[DICT] Add support for pop() method #1891
Conversation
|
Here is your checklist. Please tick items off when you have completed them or determined that they are not necessary for this pull request:
|
|
Hello again! Thank you for this new pull request 🤩. Please begin by requesting your checklist using the command |
EmilyBourne
force-pushed
the
devel-issue1886
branch
from
65f4364
to
15fbca5
Compare
|
Unfortunately your PR is not passing the tests so it is not quite ready for review yet. Let me know when it is fixed with |
|
Unfortunately your PR is not passing the tests so it is not quite ready for review yet. Let me know when it is fixed with |
![pyccel-bot[bot]](https://avatars.githubusercontent.com/in/364561?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
| @@ -843,6 +847,22 @@ def _print_ListMethod(self, expr): | |||
|
|
|||
| return f"{list_obj}.{method_name}({method_args})\n" | |||
|
|
|||
| def _print_DictMethod(self, expr): | |||
| method_name = expr.name | |||
There was a problem hiding this comment.
This code isn't tested. Please can you take a look
There was a problem hiding this comment.
/bot accept
Added for completeness. Will be tested in my next PR
|
Unfortunately your PR is not passing the tests so it is not quite ready for review yet. Let me know when it is fixed with |
|
/bot run coverage |
There was a problem hiding this comment.
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.
|
This PR is now too large. I am going to split it into 3 |
EmilyBourne
added
the
blocked
EmilyBourne
force-pushed
the
devel-issue1886
branch
from
1d43a58
to
ef211ba
Compare
|
/bot run pr_tests |
|
/bot run pr_tests |
|
Hey @pyccel/pyccel-dev ! @EmilyBourne has just created this great new pull request! Check it out and let me know what you think! |
|
Hey @pyccel/pyccel-dev ! @EmilyBourne has just created this great new pull request! Check it out and let me know what you think! |
|
Hey @yguclu, @EmilyBourne, this PR is looking pretty good. @EmilyBourne and @mustapha-belbiad 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 😄 |
pyccel-bot
bot
added
the
Ready_for_review
Add support for the
dictmethodpop()and the initialisation of a dictionary via a call to{}. Syntactic, semantic and Python printing support is added. TheDictTypedatatype is expanded to match the expected description from the docs. The classPythonDictis added to manage the initialisation. The abstract classDictMethodand the subclassDictPopare defined to handle thepop()method. Fixes #1886.Blocked by #1897