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

Allow lambdify to generate functions with out arguments #1867

Merged
merged 11 commits into from
May 8, 2024
Merged

Allow lambdify to generate functions with out arguments #1867

merged 11 commits into from
May 8, 2024

Conversation

EmilyBourne
Copy link
Member

@EmilyBourne EmilyBourne commented May 8, 2024 •
edited
Loading

Add a use_out parameter to pyccel.lambdify to avoid unnecessary memory allocation. Auto-generate a docstring for functions generated via calls to pyccel.lambdify.

@pyccel-bot
Copy link

pyccel-bot bot commented May 8, 2024

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

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

@EmilyBourne
Copy link
Member Author

EmilyBourne commented May 8, 2024 •
edited
Loading

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

yguclu
yguclu requested changes May 8, 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.

As previously discussed I am suggesting a name change

pyccel/commands/lambdify.py Outdated Show resolved Hide resolved
pyccel/commands/lambdify.py Outdated Show resolved Hide resolved
pyccel/commands/lambdify.py Outdated Show resolved Hide resolved
pyccel/commands/lambdify.py Outdated Show resolved Hide resolved
@EmilyBourne EmilyBourne marked this pull request as ready for review May 8, 2024 15:22
@github-actions github-actions bot marked this pull request as draft May 8, 2024 15:33
@pyccel-bot
Copy link

pyccel-bot bot commented May 8, 2024

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 mark as ready.

@EmilyBourne EmilyBourne marked this pull request as ready for review May 8, 2024 15:35
@github-actions github-actions bot marked this pull request as draft May 8, 2024 15:35
@pyccel-bot
Copy link

pyccel-bot bot commented May 8, 2024

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 mark as ready.

@EmilyBourne EmilyBourne marked this pull request as ready for review May 8, 2024 15:37
pyccel-bot[bot]
pyccel-bot bot approved these changes May 8, 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 May 8, 2024

@yguclu, @EmilyBourne 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 requested a review from yguclu May 8, 2024 15:57
yguclu
yguclu reviewed May 8, 2024
View reviewed changes
pyccel/commands/lambdify.py Outdated Show resolved Hide resolved
@pyccel-bot pyccel-bot bot added Ready_to_merge Approved by senior developer. Ready for final approval and merge and removed needs_initial_review labels May 8, 2024
@yguclu
Copy link
Member

yguclu commented May 8, 2024

/bot run pr_tests

pyccel-bot[bot]
pyccel-bot bot approved these changes May 8, 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 May 8, 2024
@yguclu
Copy link
Member

yguclu commented May 8, 2024

/bot run pr_tests

pyccel-bot[bot]
pyccel-bot bot approved these changes May 8, 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 May 8, 2024
@yguclu yguclu merged commit fa5ea32 into devel May 8, 2024
10 checks passed
@yguclu yguclu deleted the ebourne_lambdify_out_arg branch May 8, 2024 17:00
EmilyBourne added a commit that referenced this pull request May 13, 2024
@EmilyBourne
Allow lambdify to generate functions with out arguments (#1867)
a89774d 
Add a `use_out` parameter to `pyccel.lambdify` to avoid unnecessary
memory allocation. Auto-generate a docstring for functions generated via
calls to `pyccel.lambdify`.
EmilyBourne added a commit to pyccel/pyccel-cuda that referenced this pull request May 14, 2024
@EmilyBourne
Allow lambdify to generate functions with out arguments (pyccel#1867)
596c1c9 
Add a `use_out` parameter to `pyccel.lambdify` to avoid unnecessary
memory allocation. Auto-generate a docstring for functions generated via
calls to `pyccel.lambdify`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yguclu yguclu yguclu approved these changes

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

Assignees
No one assigned
Labels
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.
2 participants
@EmilyBourne @yguclu