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

Changing parameter names for use in Python #49

Merged
merged 9 commits into from
Feb 11, 2022

Conversation

lilyclements
Copy link
Collaborator

@lilyclements lilyclements commented Feb 11, 2022

Links to #47

@lilyclements
Copy link
Collaborator Author

@dannyparsons this is ready - thought I'd let you check before merging in case of any major issues!

Copy link
Contributor

@dannyparsons dannyparsons left a comment

Choose a reason for hiding this comment

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

Just one comment for a change.

R/climatic_summary.R Outdated Show resolved Hide resolved
@dannyparsons
Copy link
Contributor

Thanks. Does anything in climatic_summary need to change to allow it to work if elements = NULL? I haven't looked in detail at that.

@lilyclements
Copy link
Collaborator Author

Thanks. Does anything in climatic_summary need to change to allow it to work if elements = NULL? I haven't looked in detail at that.

On line 210, the elements parameter is used, and so I assume we would need to change the function for if no elements are given? Or does all_of(elements) still work even if there are no elements?

sum_data <- grp_data %>%
    dplyr::summarise(dplyr::across(dplyr::all_of(elements), 
                                   lambda_summaries, .names = names))

@dannyparsons
Copy link
Contributor

Not sure! Would be good to check. My idea was that since n() as the default summary this should work without any elements, but we probably need to add a case for this. This can be done in a separate PR though.

@dannyparsons dannyparsons merged commit d0c72d4 into IDEMSInternational:main Feb 11, 2022
@lilyclements lilyclements deleted the parameter_changes branch February 23, 2022 16:09
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.

2 participants