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.

Sign up for GitHub

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

Jump to bottom

Remove combined-yaml file #346

Open
thomas-robinson opened this issue Feb 7, 2025 · 0 comments
Open

Remove combined-yaml file #346

thomas-robinson opened this issue Feb 7, 2025 · 0 comments

Comments

@thomas-robinson
Copy link
Member

Is your feature request related to a problem? Please describe.
Creating a combined yaml is necessary to facilitate filling is reusable "variables". However, created an extra file and leaving it in a folder is not a good solution. Users will potentially create their own monolithic combined yamls, edit the combined yaml creating an untrackable situation, accidentally push the combined yaml to the repository, or any other of a myriad of possibilities.

Instead of creating a combined yaml file and outputting it into a user's directory (and leaving it there), the yaml should be constructed into a string within fre and then the internal string can be parsed. This avoids I/O to disk and leaving behind an artifact that is unwanted.

Describe the solution you'd like
Combining of the yaml to be done internally instead of by creating an external file and then reading in the file created. This is inefficient.

Describe alternatives you've considered

Additional context
singhd789 pushed a commit that referenced this issue Feb 10, 2025
#346 Do not make the combined yaml file anymore
daeb5bd 
- instead read yaml info as strings
- append strings to combine yaml information across yamls
- load combined yaml info string
- split up compile and pp classes to make script simpler and easier to follow
@singhd789 singhd789 mentioned this issue Feb 10, 2025
Merged
7 tasks
singhd789 pushed a commit that referenced this issue Feb 11, 2025
#346 Add in cleaning the yaml info
9d005b6
singhd789 pushed a commit that referenced this issue Feb 11, 2025
#346 Fix functions for merging yaml info for post-processing
5f59da1
singhd789 pushed a commit that referenced this issue Feb 11, 2025
#346 Add in an output option
213dde2 
- `output` option would dump final, cleaned yaml info to a file
singhd789 pushed a commit that referenced this issue Feb 11, 2025
#346 Update combine_yamls script
c13cf6e
singhd789 pushed a commit that referenced this issue Feb 11, 2025
#346 Rename combine_compile.py and combine_pp.py
2ed5072
singhd789 pushed a commit that referenced this issue Feb 12, 2025
#346 Merge branch 'main' of github.com:NOAA-GFDL/fre-cli into 346.no…
3078409 
…-comb-file
singhd789 pushed a commit that referenced this issue Feb 13, 2025
#346 Keep old combine-yamls tool
e425faa 
- Multiple scripts rely/use this tool
- PR would be too big if I tried to fix each one with this reworked tool
singhd789 pushed a commit that referenced this issue Feb 13, 2025
#346 Move print statements, return yaml dictionary instead
12519a6
singhd789 pushed a commit that referenced this issue Feb 13, 2025
#346 Use re-worked combine-yamls tool in configure_script_yaml.py
703310c
singhd789 pushed a commit that referenced this issue Feb 13, 2025
#346 Remove test for combined yaml
df7686f 
- this file wouldn't exist anymore
singhd789 pushed a commit that referenced this issue Feb 13, 2025
#346 combined file would not exist
c28814a
singhd789 pushed a commit that referenced this issue Feb 14, 2025
#346 Provide output option for pp tools
6ffb6fd 
- outputs a combined yaml file for other tools to use/reference
singhd789 pushed a commit that referenced this issue Feb 19, 2025
#346 Add back test to check for existence of combined yaml
1e5aafa
singhd789 pushed a commit that referenced this issue Feb 19, 2025
#346 Update scripts
1c6a681
singhd789 pushed a commit that referenced this issue Feb 19, 2025
#346 Note original combine_yamls.py kept
ce7c8f1 
- other tools also need to be changed with these PR changes so this older script is kept to not break multiple tools
singhd789 pushed a commit that referenced this issue Feb 19, 2025
#346 Add test file for combine_yamls_script.py
8639fa1
singhd789 pushed a commit that referenced this issue Feb 19, 2025
#346 Fix output option for combining compile yamls
c44e93c
singhd789 pushed a commit that referenced this issue Feb 19, 2025
#346 Fix tests for combined compile operations
0a126ad
singhd789 pushed a commit that referenced this issue Feb 19, 2025
#346 Update comment
9e73466
singhd789 pushed a commit that referenced this issue Feb 20, 2025
#346 Try to get rid of some nested if statements
fbea263
singhd789 pushed a commit that referenced this issue Feb 20, 2025
#346 Fix error messages
55766cb
singhd789 pushed a commit that referenced this issue Feb 20, 2025
#346 Try to get rid of oh so many nested if statements
66e8931
singhd789 pushed a commit that referenced this issue Feb 24, 2025
#346 Add sort_keys = False
ab83cb5
singhd789 pushed a commit that referenced this issue Feb 24, 2025
#346 Remove repetitive addition to a list
4f391e3
singhd789 added a commit that referenced this issue Feb 25, 2025
@singhd789
#346 Edit fre make code to use output from combining yaml info
d05efdc
singhd789 added a commit that referenced this issue Feb 26, 2025
@singhd789
#346 Merge branch 'main' of github.com:NOAA-GFDL/fre-cli into 346.no…
8d13fe3 
…-comb-file-fre-make
singhd789 added a commit that referenced this issue Feb 26, 2025
@singhd789
#346 Reference newer combine_yamls_script instead
1317efe 
- also fix logging messages
@singhd789 singhd789 mentioned this issue Feb 26, 2025
Open
8 tasks
singhd789 added a commit that referenced this issue Mar 3, 2025
@singhd789
#346 Merge branch 'main' of github.com:NOAA-GFDL/fre-cli into 346.no…
20aabe3 
…-comb-file-fre-make
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@thomas-robinson