Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
This release bumps the version to 0.12.8, reorganizes Looper and pipeline interface templates, and enhances the CLI and requirements for automated SRA conversion.
- Bump package version and include
--versionin both CLI andsraconvertcommands - Move pipeline and Looper YAML templates into
geofetch/templatesand update path resolution - Add (p)ypiper requirement and upgrade CI actions
Reviewed Changes
Copilot reviewed 15 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| requirements/requirements-all.txt | Added new dependency for automated SRA conversion |
| pipeline_interface_convert_v1.yaml & pipeline_interface_convert.yaml | Removed legacy pipeline interface definitions |
| manual_testing.py | Added a simple example script for manual testing |
| geofetch/templates/* | Introduced new Looper and pipeline interface templates |
| geofetch/sraconvert.py | Switched to VersionInHelpParser and added a --version flag |
| geofetch/const.py | Defined TEMPLATES_DIR and new template name constants |
| geofetch/geofetch.py | Updated all template file lookups to use TEMPLATES_DIR |
| MANIFEST.in | Updated to include everything under geofetch/templates/ |
| .looper.yaml | Added an example local Looper config file |
| .github/workflows/black.yml | Upgraded actions/checkout and actions/setup-python versions |
| geofetch/cli.py | Switched main CLI to VersionInHelpParser and embedded version |
| geofetch/_version.py | Bumped __version__ to 0.12.8 |
Comments suppressed due to low confidence (5)
requirements/requirements-all.txt:10
- The project code imports
pypiper, but the requirement is listed aspiper. It should bepypiper>=0.14.4to match the actual package name.
piper>=0.14.4
geofetch/const.py:52
- [nitpick] The constant
LOOPER_SRA_CONVERTsuggests an SRA conversion template but holds the Looper config template filename. Consider renaming toLOOPER_CONFIG_TEMPLATE_NAMEfor clarity.
LOOPER_SRA_CONVERT = "looper_config_template.yaml"
.looper.yaml:4
- [nitpick] This example config references
pipeline_interface_convert.yamlat the repo root, but that file has been removed. You may want to point to the template undergeofetch/templatesor remove this tracked example.
- /home/bnt4me/virginia/repos/geofetch/pipeline_interface_convert.yaml
manual_testing.py:1
- [nitpick] This example script lacks a module docstring or comments explaining its purpose and usage. Consider adding a top-level docstring to clarify how to run and what it demonstrates.
from geofetch import Geofetcher
geofetch/sraconvert.py:75
- This version flag references
__version__but__version__is not imported in this module, which will raise aNameError. Import it fromgeofetch._versionor adjust accordingly.
parser.add_argument(
donaldcampbelljr
approved these changes
Jul 11, 2025
| @@ -867,6 +871,8 @@ def _expand_metadata_list(self, metadata_list: list) -> list: | |||
| _LOGGER.info("Expanding metadata list...") | |||
| list_of_keys = _get_list_of_keys(metadata_list) | |||
| for key_in_list in list_of_keys: | |||
Contributor
There was a problem hiding this comment.
This is minor, but I'd recommend simplifying variable names like this to key instead of key_in_list. Given the logic, it is implied, and so, the name is perhaps a bit redundant.
| import os | ||
| import sys | ||
| from argparse import ArgumentParser | ||
| from ubiquerg import VersionInHelpParser |
Contributor
There was a problem hiding this comment.
Why did we change from using ArgumentParser to a class from ubiquerg?
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.
Changes: