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

Explicit keyword argument #225

Merged
merged 1 commit into from
Jul 2, 2024
Merged

Conversation

castelao
Copy link
Member

@castelao castelao commented Jul 2, 2024

The forward pass uses lat_lon as a positional argument, which caused an error in the PresRat implementation. Is there a good reason to don't be explicit here by using a keyword argument?

I split this from #215 to be sure that this is not breaking anything else.

There is no reason why don't be explicit here, which also gives more
freedom in the signature for new methods.

This was an issue implementing PresRat, where there was a confusion with
lat_lon and time.
@castelao castelao self-assigned this Jul 2, 2024
@castelao castelao requested review from grantbuster and bnb32 July 2, 2024 17:46
Copy link
Collaborator

@bnb32 bnb32 left a comment

Choose a reason for hiding this comment

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

This seems fine to me. I prefer explicit kwargs anyway.

Copy link
Member

@grantbuster grantbuster left a comment

Choose a reason for hiding this comment

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

fine with me!

@castelao castelao merged commit ec59bd6 into main Jul 2, 2024
12 checks passed
@castelao castelao deleted the refact/bias_correct_source_data_args branch July 2, 2024 18:22
github-actions bot pushed a commit that referenced this pull request Jul 2, 2024
There is no reason why don't be explicit here, which also gives more
freedom in the signature for new methods.

This was an issue implementing PresRat, where there was a confusion with
lat_lon and time.
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.

3 participants