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.

Sign up for GitHub

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

Jump to bottom

List support: pop method #1710

Merged
merged 62 commits into from
Feb 15, 2024
Merged

List support: pop method #1710

merged 62 commits into from
Feb 15, 2024

Conversation

mustapha-belbiad
Copy link
Contributor

@mustapha-belbiad mustapha-belbiad commented Feb 2, 2024 •
edited by yguclu

Add support for the pop() method of Python lists to the semantic stage. Add handling of that method to the Python printer. This fixes #1690.

mustapha-belbiad and others added 18 commits January 19, 2024 11:12
@mustapha-belbiad
fix documentation : correcting wrapper stage link in overview.md
2fcb2de
@mustapha-belbiad
Revert "fix documentation : correcting wrapper stage link in overview…
5975f85 
….md"

This reverts commit 2fcb2de.
@Farouk-Echaref
getting last devel updates into my branch
9e8be09
@mustapha-belbiad
Example of Lists Methods
454d0cb
@Farouk-Echaref
feat: List class_def structure & List Methods structre
82bffa9
@Farouk-Echaref
Merge branch 'devel' into feat/list_support_in_Pyccel
a23c504
@mustapha-belbiad
for merge
81927c7
@mustapha-belbiad
Merge branch 'devel' of https://github.com/pyccel/pyccel into devel
bd3b880
@mustapha-belbiad
Merge branch 'devel' into List_Methods
752554c
@Farouk-Echaref
feat: list classdef structure
8ebf4c1
@Farouk-Echaref
feat: PythonList in type inference
4d69c1a
@Farouk-Echaref
start append class doc
e270e92
@Farouk-Echaref
check on List type in classdef
ea42345
@mustapha-belbiad
add pop method for list
3e5c3f4
@mustapha-belbiad
Merge branch 'devel' of https://github.com/pyccel/pyccel into devel
e3e6c66
@mustapha-belbiad
Merge branch 'devel' into List_Methods
1f9f8a3
@mustapha-belbiad
delete test files
3c967b4
@mustapha-belbiad
Fix issues from merge step
8559b9c
@pyccel-bot
Copy link

pyccel-bot bot commented Feb 2, 2024

Hello again! Thank you for this new pull request 🤩.

Please begin by requesting your checklist using the command /bot checklist

@github-actions github-actions bot marked this pull request as draft February 2, 2024 12:05
@mustapha-belbiad
Copy link
Contributor Author

mustapha-belbiad commented Feb 2, 2024 •
edited

Here is your checklist. Please tick items off when you have completed them or determined that they are not necessary for this pull request:

  • Write a clear PR description
  • Add tests to check your code works as expected
  • Update documentation if necessary
  • Update Changelog
  • Ensure any relevant issues are linked
  • Ensure new tests are passing

@mustapha-belbiad mustapha-belbiad linked an issue Feb 2, 2024 that may be closed by this pull request
pop: removes and returns the last value from the list or the given index value. #1690
Closed
@mustapha-belbiad mustapha-belbiad self-assigned this Feb 2, 2024
@mustapha-belbiad mustapha-belbiad added Feature adding new features Containers tuples/lists/sets/maps labels Feb 2, 2024
@mustapha-belbiad mustapha-belbiad removed their assignment Feb 2, 2024
pyccel-bot[bot]
pyccel-bot bot approved these changes Feb 13, 2024
View reviewed changes
Copy link

@pyccel-bot pyccel-bot bot left a 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.

@pyccel-bot
Copy link

pyccel-bot bot commented Feb 13, 2024

@EmilyBourne, @mustapha-belbiad has been working hard and thinks that they have now replied to or fixed all your comments. Could you take another look at the PR and see if you can approve now?

@pyccel-bot pyccel-bot bot added the Ready_for_review Received at least one approval. Requires review from senior developer label Feb 13, 2024
EmilyBourne
EmilyBourne approved these changes Feb 13, 2024
View reviewed changes
Copy link
Member

@EmilyBourne EmilyBourne left a comment

Choose a reason for hiding this comment

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

Looks good to me! Well done 👍

@pyccel-bot pyccel-bot bot added Ready_to_merge Approved by senior developer. Ready for final approval and merge and removed Ready_for_review Received at least one approval. Requires review from senior developer Ready_to_merge Approved by senior developer. Ready for final approval and merge labels Feb 13, 2024
EmilyBourne
EmilyBourne reviewed Feb 13, 2024
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@yguclu
Copy link
Member

yguclu commented Feb 14, 2024

@mustapha-belbiad Please resolve the conflicts with the devel branch and update your branch.

yguclu
yguclu requested changes Feb 14, 2024
View reviewed changes
Copy link
Member

@yguclu yguclu left a comment

Choose a reason for hiding this comment

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

Good job! I have a few minor comments.

Do not forget to resolve the conflicts with devel!

AUTHORS Outdated Show resolved Hide resolved
pyccel/ast/builtin_objects/list_functions.py Outdated Show resolved Hide resolved
pyccel/ast/builtin_objects/list_functions.py Outdated Show resolved Hide resolved
pyccel/codegen/printing/pycode.py Outdated Show resolved Hide resolved
@EmilyBourne
Copy link
Member

@mustapha-belbiad Don't forget to react to the comments on your PR. See the docs:

Once your pull request has been reviewed please react to the open conversations. If you disagree you can say this, if not please leave a reference to the commit which fixes the mentioned issue. This makes the review process faster and more streamlined. Please only resolve conversations that you opened. You may think you fixed the problem, but the reviewer may disagree and leaving the discussion open makes it easier for them to verify that they agree with you. If you are reviewing then please close all conversations that you open once the problem is resolved. If you don't this can block the merge.

@yguclu
Copy link
Member

yguclu commented Feb 15, 2024

/bot run pr_tests

@pyccel-bot pyccel-bot bot added Ready_to_merge Approved by senior developer. Ready for final approval and merge and removed Ready_to_merge Approved by senior developer. Ready for final approval and merge labels Feb 15, 2024
pyccel-bot[bot]
pyccel-bot bot approved these changes Feb 15, 2024
View reviewed changes
Copy link

@pyccel-bot pyccel-bot bot left a 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.

@pyccel-bot pyccel-bot bot added Ready_to_merge Approved by senior developer. Ready for final approval and merge and removed Ready_to_merge Approved by senior developer. Ready for final approval and merge labels Feb 15, 2024
@yguclu yguclu changed the title Add support for pop method for lists in the Python translation List support: pop method Feb 15, 2024
@yguclu yguclu merged commit a563d08 into devel Feb 15, 2024
13 checks passed
@yguclu yguclu deleted the List_Methods branch February 15, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EmilyBourne EmilyBourne EmilyBourne approved these changes

@yguclu yguclu yguclu approved these changes

@Farouk-Echaref Farouk-Echaref Farouk-Echaref approved these changes

@pyccel-bot pyccel-bot[bot] pyccel-bot approved these changes

Assignees

@mustapha-belbiad mustapha-belbiad

Labels
Containers tuples/lists/sets/maps Feature adding new features Ready_to_merge Approved by senior developer. Ready for final approval and merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pop: removes and returns the last value from the list or the given index value.
4 participants
@mustapha-belbiad @EmilyBourne @yguclu @Farouk-Echaref