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

Rename Experiment.summary() argument #391

Merged
merged 4 commits into from
Oct 18, 2023

Conversation

al-rigazzi
Copy link
Collaborator

This PR addresses the shadowing of the builitin symbol format in the signature of the Experiment.summary() member function.

A lint pragma instruction could be removed, following renaming of format to style. As the argument name changes, this has to be considered and API break.

@al-rigazzi al-rigazzi changed the title Switch to style Rename Experiment.summary() argument Oct 11, 2023
@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #391 (b30cc3c) into develop (b509efd) will decrease coverage by 0.12%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #391      +/-   ##
===========================================
- Coverage    89.77%   89.66%   -0.12%     
===========================================
  Files           59       59              
  Lines         3618     3618              
===========================================
- Hits          3248     3244       -4     
- Misses         370      374       +4     
Files Coverage Δ
smartsim/experiment.py 80.95% <100.00%> (ø)

... and 1 file with indirect coverage changes

@al-rigazzi al-rigazzi added type: refactor Issues focused on refactoring existing code API break Issues that include incompatible API changes area: api Issues related to API changes labels Oct 11, 2023
@mellis13 mellis13 requested review from ashao and mellis13 October 11, 2023 18:11
@al-rigazzi al-rigazzi self-assigned this Oct 11, 2023
Copy link
Contributor

@mellis13 mellis13 left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@al-rigazzi al-rigazzi merged commit 63d1fa5 into CrayLabs:develop Oct 18, 2023
@al-rigazzi al-rigazzi deleted the summary-signature branch October 18, 2023 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API break Issues that include incompatible API changes area: api Issues related to API changes type: refactor Issues focused on refactoring existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants