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

Application.files refactor #732

Merged
merged 22 commits into from
Oct 16, 2024

Conversation

amandarichardsonn
Copy link
Contributor

@amandarichardsonn amandarichardsonn commented Sep 30, 2024

This PR adds a operations.py file that holds that holds the functionality that allows user to add files to an Application.

@amandarichardsonn amandarichardsonn marked this pull request as draft September 30, 2024 14:12
@amandarichardsonn amandarichardsonn marked this pull request as ready for review October 3, 2024 22:14
Copy link

codecov bot commented Oct 7, 2024

Codecov Report

Attention: Patch coverage is 99.31507% with 1 line in your changes missing coverage. Please review.

Project coverage is 47.32%. Comparing base (cce16e6) to head (6198a14).
Report is 19 commits behind head on smartsim-refactor.

Files with missing lines Patch % Lines
smartsim/_core/commands/command_list.py 66.66% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                  Coverage Diff                  @@
##           smartsim-refactor     #732      +/-   ##
=====================================================
+ Coverage              40.45%   47.32%   +6.87%     
=====================================================
  Files                    110      109       -1     
  Lines                   7326     6573     -753     
=====================================================
+ Hits                    2964     3111     +147     
+ Misses                  4362     3462     -900     
Files with missing lines Coverage Δ
smartsim/_core/commands/command.py 95.34% <100.00%> (-0.31%) ⬇️
smartsim/_core/generation/generator.py 100.00% <100.00%> (+2.22%) ⬆️
smartsim/_core/generation/operations/operations.py 100.00% <100.00%> (ø)
...rtsim/_core/generation/operations/utils/helpers.py 100.00% <100.00%> (ø)
smartsim/builders/ensemble.py 93.04% <ø> (ø)
smartsim/entity/application.py 90.24% <100.00%> (ø)
smartsim/entity/files.py 52.17% <ø> (-23.10%) ⬇️
smartsim/_core/commands/command_list.py 92.85% <66.66%> (ø)

... and 9 files with indirect coverage changes

Copy link
Member

@MattToast MattToast left a comment

Choose a reason for hiding this comment

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

Looks really good! Just dropping some initial feedback so that we can start ironing out some of the finer points, but otherwise this looking really good!

smartsim/_core/commands/command.py Outdated Show resolved Hide resolved
smartsim/_core/generation/generator.py Outdated Show resolved Hide resolved
smartsim/_core/generation/generator.py Outdated Show resolved Hide resolved
smartsim/_core/generation/generator.py Outdated Show resolved Hide resolved
smartsim/_core/generation/generator.py Outdated Show resolved Hide resolved
tests/test_operations.py Outdated Show resolved Hide resolved
tests/test_operations.py Outdated Show resolved Hide resolved
tests/test_operations.py Outdated Show resolved Hide resolved
tests/test_operations.py Outdated Show resolved Hide resolved
tests/test_operations.py Outdated Show resolved Hide resolved
@amandarichardsonn amandarichardsonn changed the title EntityFiles refactor Application.files refactor Oct 15, 2024
Copy link
Member

@MattToast MattToast left a comment

Choose a reason for hiding this comment

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

One last small request, but otherwise LGTM!!

smartsim/_core/generation/operations/operations.py Outdated Show resolved Hide resolved
@amandarichardsonn amandarichardsonn merged commit a39246e into CrayLabs:smartsim-refactor Oct 16, 2024
34 of 35 checks passed
@amandarichardsonn amandarichardsonn deleted the gen branch October 16, 2024 22:21
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