Automatically download files if they are missing - #225
Merged
Conversation
…erbs from the command line.
3 tasks
…he `comet` verb for completeness.
drewoldag
marked this pull request as ready for review
May 9, 2025 21:47
There was a problem hiding this comment.
Pull Request Overview
This PR implements a mechanism to automatically download missing bootstrap files when running various command‐line verbs and also ensures that the cache directory is properly passed to the underlying Sorcha observatory object. Key changes include:
- Updating CLI argument names from “c” to “config” for consistency.
- Adding calls to download missing bootstrap files in the predict, orbitfit, convert, and comet modules.
- Instantiating LayupObservatory with a specified cache_dir derived from CLI arguments.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/layup_cmdline/predict.py | Updated config argument and added bootstrap file checking |
| src/layup_cmdline/orbitfit.py | Updated config handling and added bootstrap file checking |
| src/layup_cmdline/convert.py | Updated config argument and added bootstrap file checking |
| src/layup_cmdline/comet.py | Updated config argument and added bootstrap file checking |
| src/layup_cmdline/bootstrap.py | Updated config argument name |
| src/layup/utilities/data_processing_utilities.py | Updated LayupObservatory to accept a cache_dir argument |
| src/layup/utilities/bootstrap_utilties/download_utilities.py | Added helper function to download missing files |
| src/layup/predict.py | Passed cache_dir to LayupObservatory in predict function |
| src/layup/orbitfit.py | Passed cache_dir to LayupObservatory in orbitfit function |
Comments suppressed due to low confidence (2)
src/layup_cmdline/predict.py:287
- [nitpick] The block of code for loading and validating the configuration file is repeated in multiple CLI modules. Consider refactoring this logic into a shared utility function to reduce duplication.
configs = LayupConfigs(); if args.config: find_file_or_exit(args.config, "-c, --config")
src/layup/utilities/bootstrap_utilties/download_utilities.py:62
- The function uses _decompress as a processor in the partial function, but no definition or import for _decompress is provided in this diff. Please ensure that _decompress is properly defined or imported.
fetch_partial = partial(retriever.fetch, processor=_decompress, progressbar=True)
wilsonbb
approved these changes
May 9, 2025
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatically download files if they are missing when running verbs from the command line. When creating
SorchaObservatoryobject, request that the layup cache files are used.Fixes #126 #108 .
Describe your changes.
Two big changes here
convert,predict, ororbitfitfrom the commandline. If anything is missing, we download it (and decompress if needed). This should fix issue automatically download the files layup bootstrap does if it's not been run before one of the other layup verbs runs #126.LayupObservatoryobject, we instantiate aSorchaObservatoryobject under the hood. The SorchaObservatory can take a cache directory as input, which we were not passing. Now we will, so the SorchaObservatory object will look in thelayupcache directory for the files it needs. This should fix issuesorcha<->layupcaching conflict #108.Review Checklist for Source Code Changes