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

List support/insert method #1727

Merged
merged 123 commits into from
Feb 21, 2024
Merged

List support/insert method #1727

merged 123 commits into from
Feb 21, 2024

Conversation

Farouk-Echaref
Copy link
Contributor

@Farouk-Echaref Farouk-Echaref commented Feb 13, 2024

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

Commit Summary

  • Add a class representation for the insert() method, inheriting from PyccelInternalFunction. The constructor ensures compatibility between the datatypes of the insert() argument and the list for homogenous lists.
  • Update the _class_type attribute for methods that return None
  • Add the python printer _print_ListInsert() to generate the appropriate Python representation of the insert() method.
  • Add success and error tests for the insert() method involving many scenarios where the function could be used.

…to raise proper error message in case or dtype conflicts
@Farouk-Echaref
Copy link
Contributor Author

/bot run docs pylint pyccel_lint spelling

@Farouk-Echaref
Copy link
Contributor Author

/bot run linux macosx windows

@Farouk-Echaref
Copy link
Contributor Author

/bot run coverage

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.

@Farouk-Echaref Farouk-Echaref marked this pull request as ready for review February 19, 2024 13:46
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 the Ready_for_review Received at least one approval. Requires review from senior developer label Feb 19, 2024
@pyccel-bot
Copy link

pyccel-bot bot commented Feb 19, 2024

@EmilyBourne, @Farouk-Echaref 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?

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. 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 labels Feb 19, 2024
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.

Looks good! I have some minor comments

pyccel/ast/builtin_methods/list_methods.py Outdated Show resolved Hide resolved
pyccel/ast/builtin_methods/list_methods.py Outdated Show resolved Hide resolved
pyccel/ast/builtin_methods/list_methods.py Outdated Show resolved Hide resolved
pyccel/ast/builtin_methods/list_methods.py Outdated Show resolved Hide resolved
pyccel/ast/class_defs.py Outdated Show resolved Hide resolved
@github-actions github-actions bot marked this pull request as draft February 21, 2024 10:15
@pyccel-bot
Copy link

pyccel-bot bot commented Feb 21, 2024

@Farouk-Echaref, @yguclu has a few questions/comments about your code. Can you go through and see if you agree with them. If not go ahead and explain why. Once you've adressed all the comments let me know with /bot mark as ready and we will see if we can get approval.

@pyccel-bot pyccel-bot bot removed the Ready_to_merge Approved by senior developer. Ready for final approval and merge label Feb 21, 2024
@yguclu yguclu marked this pull request as ready for review February 21, 2024 14:21
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 the Ready_to_merge Approved by senior developer. Ready for final approval and merge label Feb 21, 2024
@yguclu yguclu merged commit e371f27 into devel Feb 21, 2024
11 checks passed
@yguclu yguclu deleted the List_Support/insert_method branch February 21, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

insert: inserts a given element at a given index in a list.
4 participants