-
Notifications
You must be signed in to change notification settings - Fork 13
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
Utility to remove duplicate arguments for calls and callees #367
Conversation
Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/367/index.html |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #367 +/- ##
==========================================
+ Coverage 95.52% 95.54% +0.01%
==========================================
Files 186 188 +2
Lines 38956 39105 +149
==========================================
+ Hits 37214 37363 +149
Misses 1742 1742
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
First of all, apologies for the appallingly long time it took me to review this!
This is a really nice and useful utility and a prime example of the inter-procedural way of working in Loki. I think the implementation could be improved in a few places to make it more pythonic and I've left comments in the code.
From a functionality point of view, I left a question whether this sufficiently safeguards against accidental name clashes when enabling the rename_common
mode.
loki/transformations/utilities.py
Outdated
rename_kwarguments(relevant_calls, rename_common_map_routine) | ||
|
||
|
||
def modify_variable_declarations(routine, remove_symbols=(), rename_symbols=None): |
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.
d89f737
to
eb5831c
Compare
…new file/module 'routine_signatures'
eb5831c
to
949bf65
Compare
@reuterbal all your comments and requested changes should be addressed except for the |
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.
Many thanks for implementing the suggested changes. I think this looks ready now!
Transform/convert e.g.,
to