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

style[cartesian]: Remove unused optional keyword arguments #1805

Merged
merged 2 commits into from
Jan 21, 2025

Conversation

romanc
Copy link
Contributor

@romanc romanc commented Jan 20, 2025

Description

These two keyword arguments aren't used in the function. Since the two functions have a "catch-all" *kwargs at the end of the argument list, we can safely delete the unused keyword arguments.

Requirements

  • All fixes and/or new features come with corresponding tests.
    mypy and/or existing tests would complain if these changes weren't appropriate
  • Important design decisions have been documented in the appropriate ADR inside the docs/development/ADRs/ folder.
    N/A

These two keyword arguments aren't used in the function. Since the two
functions have a "catch-all" `*kwargs` at the end of the argument list,
we can safely delete the unused keyword arguments.
Copy link
Contributor

@FlorianDeconinck FlorianDeconinck left a comment

Choose a reason for hiding this comment

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

LGTM.

I really don't like the kwargs, but it's used throughout

Copy link
Contributor

@twicki twicki left a comment

Choose a reason for hiding this comment

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

LGTM

No need to pass down the loop order of a `VerticalLoop`. It is never
used/access inside `VerticalLoopSection`s or `HorizontalExecution`s.
@romanc
Copy link
Contributor Author

romanc commented Jan 20, 2025

(please squash the two commit into one when merging)

@havogt havogt merged commit 8eae147 into GridTools:main Jan 21, 2025
21 checks passed
@romanc romanc deleted the romanc/remove-unused-arguments branch January 22, 2025 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants