Jns updates #97
Jns updates #97
Conversation
- moved process function args to host_config and index/geom_config - moved common functions and classes to common.py
- added task_file_only arg to proces_index() - added util.append_txt_file()
- removed index._udpate_task_file() and util.delete_task_file()
- added GO_0xxx_geometry_cloud.py
- added --index_tree argument to index code - restored task file functionality
- added cloud config and startup files - added --index_tree argument to index code - restored task file functionality
- Updated unit tests for new column names - removed night_override test from ring template - added $ONCE(night_override = False) to ring_summary_columns.lbl - added 'columns' argument to PdsTemplate call - restored sky_summary.lbl
- Improved the procedures described in __init__.py - removed procedure instructions from all driver scripts - removed procedure instructions from all label templates
- change INTERCEPT_TIME null value to "NA" - Add procedures for adding columns in __init__.py and README.md - Add Oxford comma to supplemental template
body_diameter_in_pixels to single value - build override dicts, leave unused for now - add valid_minimum/maximum to format dict
- implement new rules for body selection
- fix sclk problem - create final mission table
- change EXCLUSIONS to EXCEPTIONS
- add correct system name to body table - updates to mission table
- added zmin, zmax, scaled args to backplane/limb.py/limb_clock_angle - fixed bugs in _get_range_mod360
- added NULL_CONSTANT for RING_POLE_CLOCK_ANGLE - added valid min, max to center_coordinate in format dict
- move glob from geometry/index_config to host_config - added glob arg to geom code to glob files. - changed glob arg to index_glob in geometry code
- fix bugs arising from non-pointed obsevation
…) in index_support.py - add util.sets_as_lists() - replace descriptions for EDGE_ON_RING_ALTITUDE - in galileo/ssi/__init__.py, replace SpiceFrame w SpiceType1Frame with tol=40 - in galileo/ssi/__init__.py, add "NONE" telemetry mode - add init_logger() to common.py - call _init_logger() form geometry_support
- update geometry glob pattern to exclude only G files. - remove util.sets_as_lists() - replace util.as_lists(Pds3Label(path).dict) with PdsLabel.from_file(path).as_dict()
- add metadata_tree arg - add util.select_dir
- change input_tree arg to volume_tree - add metadata_tree arg - add util.select_dir
- add post processor for geom columns - add no_metadata arg to get_common_args()
WalkthroughCore API and initialization moved from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 51
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
metadata_tools/hosts/GO_0xxx/geometry_config.py (1)
206-223: Unreachable code: entire block after line 206 is dead.The unconditional
return dict["TARGET_NAME"]at line 206 makes lines 208-223 unreachable. If this logic is needed, remove the early return; otherwise, remove the dead code.Option 1: Remove early return to enable logic
def target_name(dict): ... - return dict["TARGET_NAME"] - target = dict["TARGET_NAME"] if target != "SKY": return target ...Option 2: Remove dead code
def target_name(dict): ... return dict["TARGET_NAME"] - - target = dict["TARGET_NAME"] - if target != "SKY": - return target - ... - try: - return col.CIMS_TARGET_ABBREVIATIONS[abbrev] - except KeyError: - return targetmetadata_tools/label_support.py (1)
42-54: Missing null check forhost_template_path.Line 42 calls
host_template_path.parentwithout validating thathost_template_pathis notNone. If a caller passesNone, this will raise anAttributeError. Consider adding validation or documenting the requirement.Proposed fix
+ if host_template_path is None: + raise ValueError("host_template_path is required") template_dir = host_template_path.parentmetadata_tools/column/COLUMNS_BODY.py (1)
1-6: Missingdefsandutilimports cause NameError.The file uses
defs.BODYX,defs.BODY_NAMES,util.replacement_dict(), andutil.replace()throughout but never imports these modules. This will cause aNameErrorat runtime when the module is first loaded (during the list definitions at lines 38-76).Proposed fix
################################################################################ # COLUMNS_BODY.py: Column definitions for body geometry tables ################################################################################ import oops + +import metadata_tools.defs as defs +import metadata_tools.util as util
🤖 Fix all issues with AI agents
In @.gitignore:
- Line 127: The commented-out ignore rule "#*venv*/" allows Python virtual
environment directories to be committed; either restore the ignore by
uncommenting the pattern to "*venv*/" or, if committing venvs is intentional,
add a clear justification comment adjacent to the pattern (or a short note in
README/CONTRIBUTING) that explains the new environment management strategy and
why venvs must be tracked; update the .gitignore entry or documentation
accordingly and ensure the change is deliberate, not accidental.
In @metadata_tools/column/COLUMNS_RING.py:
- Around line 95-96: The file COLUMNS_RING.py references defs.BODYX and
defs.BODY_NAMES but never imports defs, causing a NameError; add an import for
the defs module at the top of the file (e.g., use a package-relative import like
"from . import defs" or an absolute import like "import metadata_tools.defs as
defs") so references to defs in functions/tuples such as the entries using
"pole_clock_angle", "pole_position_angle" and any other uses of defs.BODYX /
defs.BODY_NAMES resolve correctly.
In @metadata_tools/common.py:
- Line 152: The class definition for Table unnecessarily inherits from object;
update the class declaration by removing the explicit inheritance so that "class
Table(object):" becomes "class Table:"; ensure any other similar Python 3-style
classes in the same file follow the same pattern if present and run tests/lint
to confirm no style issues remain.
- Around line 59-60: Replace the explicit loop that yields each item from
task_list with the more concise generator forwarding; locate the spot where "for
task in task_list: yield task" (the generator using the task_list variable) and
change it to use "yield from task_list" so the generator simply delegates to the
task_list iterable.
- Around line 88-90: The explicit file.close() after the with-open block is
redundant and should be removed: in the block that opens task_file and calls
json.dump(task_list, file, indent=2) (referencing task_file, task_list,
json.dump and the variable file), drop the trailing file.close() so resource
cleanup relies on the with context manager.
- Line 34: The call uses the instance method incorrectly: replace the
class-style call FCPath.unlink(path, missing_ok=True) with an instance call on
the Path object by invoking path.unlink(missing_ok=True) so the method receives
the instance as self; update the call site where FCPath.unlink is used to call
path.unlink instead (reference: FCPath.unlink and the local variable path).
In @metadata_tools/cumulative_support.py:
- Line 129: The variable index_type returned by util.parse_template_name is
unused; update the unpacking in cumulative_support (the line assigning host,
index_type, template_dir) to mark it as intentionally unused by renaming it with
a leading underscore (e.g., _index_type or _), so the tuple unpack remains
correct but follows Python convention for unused variables.
In @metadata_tools/defs.py:
- Line 5: Remove the unused import by deleting the line that imports the inspect
module (the "import inspect" statement) from metadata_tools/defs.py so the file
no longer contains an unused import flagged by Flake8 F401.
In @metadata_tools/geometry_support.py:
- Line 244: Replace deprecated logger.warn calls with logger.warning and switch
usages that log tracebacks to logger.exception so exception info is captured
automatically; specifically, update the logger.warn(str(e)) calls and any
logger.error(traceback.format_exc()) in metadata_tools/geometry_support.py (the
occurrences around the logger variable at the blocks noted near lines 244, 248,
and 254) to use logger.warning(str(e)) for non-exception warnings and
logger.exception("Descriptive message") when inside an except: block to log the
traceback.
- Around line 454-455: The function signature currently uses a mutable default
for the parameter tiles (tiles=[]); change the default to None (tiles=None) and
inside the function initialize tiles = [] if tiles is None to avoid shared-state
across calls; do this for every function/definition that uses tiles=[]
(including the other occurrence noted in the diff) so that the parameter is
consistently initialized inside the function body rather than at definition
time.
- Line 1274: The unpacking of FORMAT_DICT entries in the assignment "(_,_,_,_,_,
null_value, valid_minimum, valid_maximum) = format" expects 8 items but
FORMAT_DICT tuples contain 10 elements; update the unpacking in the code that
handles the variable named format (from FORMAT_DICT) to match all 10 elements
(e.g., provide two additional placeholder names or use tuple slicing/extended
unpacking) so the number of targets equals the number of elements and the
correct fields (including null_value, valid_minimum, valid_maximum) are assigned
from the FORMAT_DICT entries.
- Line 1541: The for-loop over volume_tree.walk() declares an unused variable
named files; rename it to _files (or _ ) to mark it as intentionally unused and
avoid linter warnings. Update the loop header "for root, dirs, files in
volume_tree.walk():" to "for root, dirs, _files in volume_tree.walk():" (or "for
root, dirs, _ in volume_tree.walk():") and adjust any references if there are
none—no other code changes required in the function that uses
volume_tree.walk().
- Around line 1577-1590: The task file is being written inside the per-volume
loop because com.write_task_file(task_file) is indented within the loop; move
the call so it executes after the loop completes. Specifically, ensure the block
that iterates volumes (the for loop that calls com.add_task(...) or creates
Suite and calls suite.create(...)) ends before calling
com.write_task_file(task_file) so the full task list accumulated by com.add_task
is flushed once; keep the task_list_only conditional logic (task_list_only,
com.add_task, Suite.create) unchanged other than adjusting indentation so
com.write_task_file runs outside the loop.
In @metadata_tools/hosts/GO_0xxx/gcp_geometry_config.yml:
- Around line 1-10: The GCP budget cap is too low for the requested fleet: with
region and instance family set to us-central1 and instance-types "n2-" targeting
n2-standard-8 (8 vCPU, 32GB) five instances will exceed
max-total-price-per-hour: 1.00; fix by either lowering max-instances to 2–3,
switching instance-types to a smaller SKU such as n2-standard-4 (adjust max-cpu
and min-memory-per-cpu accordingly), or increasing max-total-price-per-hour to
cover ~0.39/hour per n2-standard-8 (so ~2.0+/hour for 5 instances); update the
gcp config keys max-instances, instance-types (and related
max-cpu/min-memory-per-cpu if changing size), or max-total-price-per-hour to
reflect the chosen option.
In @metadata_tools/hosts/GO_0xxx/gcp_geometry_startup.sh:
- Line 9: The exported MY_WORKER_DEST_BUCKET value is a placeholder
(gs://my-bucket/results); update the startup script to use a real bucket or make
it configurable by reading from an external environment variable or argument
instead of hardcoding: change the export of MY_WORKER_DEST_BUCKET to derive its
value from an incoming env var (or a passed-in CLI param) and document the
required variable, ensuring any code that references MY_WORKER_DEST_BUCKET (the
export statement) will use the provided bucket path at runtime.
- Around line 3-5: The two bare cd commands (first `cd` to home and then `cd
rms-metadata-tools` after `git clone`) can fail silently; update the script to
verify each directory change succeeded and abort with an error message if not —
e.g., after attempting to change directory, check the command exit status and
exit non‑zero with a clear log on failure, and after `git clone` ensure the
`rms-metadata-tools` directory exists before attempting to cd into it;
alternatively enable strict failure handling at the top of the script (e.g., set
-e) and add explicit checks around the `cd` operations to prevent subsequent
commands running in the wrong directory.
- Line 1: Add a shebang line (#!/bin/bash) as the very first line of the script,
before the existing "apt-get update -y" command, so the file runs with the
correct interpreter; then save with Unix line endings and set the executable bit
(chmod +x) so the script can be executed.
- Line 10: The startup line runs GO_0xxx_geometry_cloud.py from the wrong
working directory and leaves RMS_METADATA unquoted; before invoking the script
(the python3 GO_0xxx_geometry_cloud.py invocation), change into the directory
that contains GO_0xxx_geometry_cloud.py (so the script can be executed relative
to its location) and update the invocation to quote the variables (use
"$RMS_METADATA/GO_0xxx/" for both arguments or "./GO_0xxx_geometry_cloud.py"
after cd) to prevent word splitting.
In @metadata_tools/hosts/GO_0xxx/gcp_index_startup.sh:
- Line 14: The export of MY_WORKER_DEST_BUCKET currently hardcodes a placeholder
value ("gs://my-bucket/results"); update the startup script so
MY_WORKER_DEST_BUCKET is set to the real GCS destination or made configurable
(e.g., read from an existing environment variable, runtime parameter, or config
file) instead of the placeholder; locate the export MY_WORKER_DEST_BUCKET line
in the gcp_index_startup.sh script and replace the literal bucket path with a
reference to the real bucket (or a fallback mechanism) and ensure any
documentation/CI that sets this variable is updated accordingly.
- Around line 15-17: The script gcp_index_startup.sh currently runs
GO_0xxx_geometry.py while GO_0xxx_index_cloud.py is commented out and a test
volume GO_0017 is hardcoded; either make this script actually perform indexing
by uncommenting and invoking GO_0xxx_index_cloud.py (and remove or parameterize
any geometry invocation), or rename the file to gcp_geometry_startup.sh if its
intent is geometry and keep GO_0xxx_geometry.py; in either case remove the
hardcoded GO_0017 from the GO_0xxx_geometry.py invocation and accept the volume
as a positional argument or iterate over a provided list so the script is
reusable.
- Line 1: The script gcp_index_startup.sh is missing a shebang; add a top-line
interpreter directive (#!/bin/bash) as the very first line of
gcp_index_startup.sh so the script runs with bash when executed (mirror the
change made in gcp_geometry_startup.sh).
- Around line 8-16: The script uses multiple bare cd commands (the initial cd,
cd rms-metadata-tools, and cd hosts/GO_0xxx) with no error handling; add robust
failure checks by enabling strict mode (e.g., set -euo pipefail) near the top
and/or change each cd to either pushd/popd or to guarded forms like cd <dir> ||
{ echo "Failed to cd to <dir>"; exit 1; } so the script aborts if a directory
change fails and subsequent commands do not run in the wrong location.
- Around line 2-5: Remove the redundant commented apt-get error lines in
gcp_index_startup.sh (the commented lines starting with "# Reading package
lists... Done" and the two "# E: ..." entries); simply delete those comment
lines so the script contains only relevant commands and comments.
In @metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py:
- Line 16: The file imports os and sys via "import os, sys" but os is unused;
edit the import in GO_0xxx_geometry_cloud.py to remove the unused os symbol
(leave only sys or other actually used modules), ensuring there are no other
references to os in functions/classes in that module such as any top-level
imports or helper functions.
- Around line 26-39: The process_task function declares an unused parameter
task_id (required by the Worker interface); rename it to _task_id (i.e., change
the parameter name in the process_task signature) to indicate intentional
non-use and silence linters, leaving the rest of the function (process_tables
call and return) unchanged; ensure any references (if any) are updated to the
new name.
In @metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py:
- Line 32: Remove the unused os import from the top-level import statement in
GO_0xxx_index_cloud.py: modify the line that currently imports "os, sys" to only
import the modules actually used (leave "sys" if it's used elsewhere) so that
"os" is no longer imported anywhere in the file; search for any references to os
in the file first and only remove the import if there are none.
- Around line 42-52: The function process_task has an unused parameter task_id
required by the Worker interface; rename or prefix it with an underscore (e.g.,
_task_id) in the process_task signature to signal it is intentionally unused,
leaving task_data and worker_data as-is and ensuring no other references to
task_id remain; update any type hints or docstrings if present to match the new
name.
- Around line 60-75: Prefix the unused template_dir with an underscore (rename
to _template_dir), and stop reading the private attribute worker._data.args;
instead parse or capture the args via the public parser before constructing
Worker and use that parsed args when calling process_index (e.g., parsed_args =
parser.parse_args(sys.argv[1:]); pass parsed_args into Worker via
args=parsed_args and then call process_index(..., args=parsed_args, ...)).
Ensure references are to util.parse_template_name, get_args, Worker,
process_index, and do not access the worker._data private attribute.
In @metadata_tools/hosts/GO_0xxx/host_config.py:
- Around line 9-11: Remove the extra blank line after the template_name
assignment to keep file formatting consistent; locate the template_name variable
(template_name = 'GO_0xxx_supplemental_index') and ensure there is only a single
trailing newline following that line.
In @metadata_tools/hosts/GO_0xxx/templates/GO_0xxx_sky_summary.lbl:
- Around line 58-59: The DESCRIPTION string in GO_0xxx_sky_summary.lbl contains
an extra comma ("data file's, label"); remove the errant comma so the text reads
"data file's label" within the DESCRIPTION value to correct the grammar.
- Around line 38-42: The template uses an undefined variable TABLE_TYPE in the
conditional; replace references to TABLE_TYPE with the defined variable
index_type (i.e., change the conditional to check index_type == 'CUMULATIVE') or
explicitly define TABLE_TYPE from index_type at the top of the template (e.g.,
set TABLE_TYPE = index_type) so the $IF/...$END_IF block evaluates correctly.
In @metadata_tools/hosts/GO_0xxx/tests/test_geometry.py:
- Around line 8-9: The two commented import lines ("#metadata_tools.util as
util" and "#import metadata_tools.hosts.GO_0xxx.host_config as config") violate
PEP8 comment spacing; either delete them if unused or reformat them as proper
comments by adding a space after the hash (e.g., "# metadata_tools.util as util"
and "# import metadata_tools.hosts.GO_0xxx.host_config as config") so they
comply with E265; update the test file accordingly.
In @metadata_tools/index_support.py:
- Line 7: Remove the unused imports flagged by static analysis: delete the
top-level import of json and the import statements that bring in
metadata_tools.defs, pdstable, and FileCache from filecache; specifically remove
the names imported in the module import block so only actually used modules
remain (verify no references to json, defs, pdstable, or FileCache in functions
like any in this file before removing). After removing those import lines, run
tests or lint to confirm no missing-name errors and re-run the static analyzer
to ensure the warnings gone.
- Line 567: The call to the deprecated logger.warn needs to be replaced with
logger.warning: locate the usage of logger.warn('Unused columns: %s', unused) in
the index_support module (symbol: logger.warn) and change it to
logger.warning('Unused columns: %s', unused) so the standard logging API is
used; no other behavior changes are required.
- Line 524: The loop over volume_tree.walk() uses an unused variable named
files; rename it to _files to signal intentional non-use (i.e., change "for
root, dirs, files in volume_tree.walk()" to "for root, dirs, _files in
volume_tree.walk()") so linters and readers know the variable is intentionally
unused while leaving root and dirs intact.
- Around line 549-563: The task file is being written inside the per-volume loop
which causes repeated overwrites when task_list_only is True; move the
com.write_task_file(task_file) call out of the loop so it executes once after
all volumes are processed (check the loop guarded by task_list_only and call
com.write_task_file(task_file) after that loop), referencing the existing
task_list_only flag, the com.write_task_file method, and the task_file variable
to locate where to change.
In @metadata_tools/label_support.py:
- Around line 13-16: The create function signature allows use_global_template as
a positional boolean; change the signature of create so that use_global_template
(and system/table_type if desired) are keyword-only by adding a positional-only
separator (e.g. insert a * after host_template_path) so callers must call
create(..., use_global_template=True). Update any call sites invoking create
with positional booleans to use keyword arguments.
In @metadata_tools/util.py:
- Around line 5-8: The file imports the same module twice; remove the redundant
import of the re module (the duplicate second "import re" in the import block)
so only a single "import re" remains at the top of the file, leaving all other
imports (numpy as np, cspyce) unchanged.
- Around line 418-424: The file write loop is iterating characters because
content is a single string; replace the character-by-character loop in the
with-open block by writing the entire string at once (e.g., file.write(content))
or by iterating over content.splitlines(keepends=True) to write real lines;
update the with open(...) as file: section that references content to use one of
these correct approaches and keep the file mode ("a") as intended.
- Around line 28-30: The variable local_table_path is assigned via
label_path.with_suffix('.tab').retrieve() but never used; either remove that
assignment or make the intent explicit by renaming it to _local_table_path or
adding a comment explaining the side-effect of caching the .tab file. Update the
code around local_label_path and the return of
pdstable.PdsTable(local_label_path) so you either delete the unused
local_table_path line or keep it with a clear comment like "ensure .tab file is
cached locally" or rename to _local_table_path to indicate intentional unused
value.
- Line 14: The import metadata_tools.common as com in util.py is unused; remove
that import statement to clean up dead code and avoid linter warnings. Locate
the import line "import metadata_tools.common as com" near the top of util.py
and delete it, then run tests/lint to ensure no other references to com exist
and adjust if any are found.
- Around line 597-612: Update the function docstring to match the actual
4-element return tuple: the function returns a NamedTuple with four fields
(e.g., primary (str), secondaries (list), selections (list), extra (list))
instead of three; make the docstring reflect the same field names and types as
the actual returns (see the fail tuple ('', [], [], []) and the returned
(row[2], row[3], row[4], row[5])), so readers understand the fourth element
returned by this function.
- Around line 33-46: The function select_dir has a logic bug where dir is
unconditionally overwritten and its docstring is wrong; update select_dir to:
accept parameters tree, col, vol as Path-like values, correct the docstring to
describe these params and return value, and implement logic that returns
tree/col when tree.parts[-1] != col otherwise returns tree/vol (avoid using the
built-in name dir—use result or path variable instead); ensure no unconditional
reassignment and add a unit test for both branches.
- Around line 569-583: The code in obs_excluded uses an undefined name config
(referenced in the getattr(config, exception) call) which will raise NameError;
fix by either importing the config module at top of this module and using it
directly, or change obs_excluded to accept a config parameter and use that
parameter in the getattr call (update all callers accordingly); ensure you
reference the existing get_observation_id and the exception.isidentifier() check
when updating the function signature or adding the import so the behavior
remains the same.
In @README.md:
- Around line 95-98: The indented code block containing "key__<NAME>(label_path,
label_dict)" should be converted to a fenced code block for consistency and to
satisfy markdownlint MD046; replace the indented block with a triple-backtick
fenced block, add the language hint (e.g., ```python) before the line, include
the key__<NAME>(label_path, label_dict) line, and close with a trailing triple
backtick so the snippet for the key__<NAME> symbol is rendered consistently.
In @requirements-cloud.txt:
- Around line 1-12: In requirements-cloud.txt the dependency is misspelled as
"json_stream" (which is not on PyPI); update that entry to "json-stream"
(hyphen) and, while here, consider adding minimum version constraints for
unpinned packages like "numpy", "rms-oops", "rms-julian", "rms-pdsparser",
"rms-pdstable", "rms-vicar", "rms-pdstemplate", and "fortranformat" to improve
reproducibility.
In @tests/unittester_support.py:
- Around line 82-84: Remove the commented-out debugging lines that print
variables and launch IPython (the lines containing "print(nullval, key, val)"
and the block checking "'RIGHT_ASCENSION' in key" with "from IPython import
embed; print('+++++++++++++'); embed()"); simply delete these commented lines
from tests/unittester_support.py so no leftover debug stubs remain.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (51)
.flake8.gitignoreREADME.mdmetadata_tools/__init__.pymetadata_tools/column/COLUMNS_BODY.pymetadata_tools/column/COLUMNS_RING.pymetadata_tools/columns.pymetadata_tools/common.pymetadata_tools/cumulative_support.pymetadata_tools/defs.pymetadata_tools/geometry_support.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_cumulative.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_geometry.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_index.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.pymetadata_tools/hosts/GO_0xxx/addition_tasks.jsonmetadata_tools/hosts/GO_0xxx/gcp_geometry_config.ymlmetadata_tools/hosts/GO_0xxx/gcp_geometry_startup.shmetadata_tools/hosts/GO_0xxx/gcp_index_config.ymlmetadata_tools/hosts/GO_0xxx/gcp_index_startup.shmetadata_tools/hosts/GO_0xxx/geometry_config.pymetadata_tools/hosts/GO_0xxx/gs:/rms-jspitale/metadata_test/GO_0xxx/GO_0016/GO_0016_supplemental_index.lblmetadata_tools/hosts/GO_0xxx/gs:/rms-jspitale/metadata_test/GO_0xxx/GO_0016/GO_0016_supplemental_index.tabmetadata_tools/hosts/GO_0xxx/host_config.pymetadata_tools/hosts/GO_0xxx/index_config.pymetadata_tools/hosts/GO_0xxx/index_tasks.jsonmetadata_tools/hosts/GO_0xxx/pymetadata_tools/hosts/GO_0xxx/rms-metadata-toolsmetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_body_summary.lblmetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_ring_summary.lblmetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_sky_summary.lblmetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_supplemental_index.lblmetadata_tools/hosts/GO_0xxx/tests/test_geometry.pymetadata_tools/hosts/GO_0xxx/tests/test_index.pymetadata_tools/index_support.pymetadata_tools/label_support.pymetadata_tools/stage.shmetadata_tools/templates/body_summary_columns.lblmetadata_tools/templates/edge_on_definition.lblmetadata_tools/templates/header.lblmetadata_tools/templates/inventory.lblmetadata_tools/templates/ring_summary_columns.lblmetadata_tools/templates/ring_summary_details.lblmetadata_tools/templates/sky_summary.lblmetadata_tools/templates/sky_summary_columns.lblmetadata_tools/util.pyrequirements-cloud.txtrequirements.txttests/test_geometry.pytests/unittester_support.py
💤 Files with no reviewable changes (4)
- metadata_tools/templates/header.lbl
- metadata_tools/templates/sky_summary.lbl
- metadata_tools/stage.sh
- metadata_tools/hosts/GO_0xxx/tests/test_index.py
🧰 Additional context used
🧬 Code graph analysis (12)
metadata_tools/label_support.py (1)
metadata_tools/util.py (1)
get_template_name(74-84)
metadata_tools/hosts/GO_0xxx/tests/test_geometry.py (1)
tests/unittester_support.py (1)
bounds(51-88)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py (3)
metadata_tools/geometry_support.py (2)
process_tables(1485-1590)get_args(1441-1482)metadata_tools/util.py (1)
parse_template_name(87-107)metadata_tools/common.py (1)
task_source(52-60)
tests/test_geometry.py (1)
tests/unittester_support.py (1)
bounds(51-88)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py (4)
metadata_tools/index_support.py (2)
process_index(571-619)get_args(451-476)metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py (2)
process_task(26-39)main(42-70)metadata_tools/util.py (1)
parse_template_name(87-107)metadata_tools/common.py (1)
task_source(52-60)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry.py (1)
metadata_tools/geometry_support.py (1)
process_tables(1485-1590)
metadata_tools/hosts/GO_0xxx/GO_0xxx_cumulative.py (2)
metadata_tools/cumulative_support.py (1)
create_cumulative_indexes(121-169)tests/unittester_support.py (1)
exclude(30-48)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index.py (1)
metadata_tools/index_support.py (1)
process_index(571-619)
metadata_tools/cumulative_support.py (4)
metadata_tools/common.py (2)
get_logger(41-43)get_common_args(98-147)metadata_tools/index_support.py (1)
create(107-154)metadata_tools/label_support.py (1)
create(13-72)metadata_tools/util.py (2)
parse_template_name(87-107)get_volume_glob(251-266)
metadata_tools/common.py (4)
metadata_tools/geometry_support.py (2)
write(1360-1371)create(1374-1433)metadata_tools/util.py (1)
write_txt_file(348-383)metadata_tools/index_support.py (1)
create(107-154)metadata_tools/label_support.py (1)
create(13-72)
metadata_tools/util.py (3)
metadata_tools/common.py (1)
write(196-222)metadata_tools/geometry_support.py (1)
write(1360-1371)tests/unittester_support.py (1)
match(13-26)
metadata_tools/geometry_support.py (3)
metadata_tools/util.py (3)
get_primary(586-612)parse_template_name(87-107)select_dir(33-46)metadata_tools/common.py (4)
get_logger(41-43)Table(152-222)write(196-222)get_common_args(98-147)metadata_tools/index_support.py (3)
add(157-197)create(107-154)get_args(451-476)
🪛 Flake8 (7.3.0)
metadata_tools/label_support.py
[error] 14-14: continuation line over-indented for visual indent
(E127)
[error] 15-15: continuation line over-indented for visual indent
(E127)
[error] 16-16: continuation line over-indented for visual indent
(E127)
metadata_tools/hosts/GO_0xxx/tests/test_geometry.py
[error] 8-8: block comment should start with '# '
(E265)
[error] 9-9: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py
[error] 16-16: 'os' imported but unused
(F401)
[error] 25-25: block comment should start with '# '
(E265)
[error] 41-41: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py
[error] 32-32: 'os' imported but unused
(F401)
[error] 41-41: block comment should start with '# '
(E265)
[error] 54-54: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry.py
[error] 44-44: 'host_init' imported but unused
(F401)
metadata_tools/hosts/GO_0xxx/GO_0xxx_cumulative.py
[error] 30-30: 'host_init' imported but unused
(F401)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index.py
[error] 26-26: 'host_init' imported but unused
(F401)
metadata_tools/index_support.py
[error] 7-7: 'json' imported but unused
(F401)
[error] 11-11: 'metadata_tools.defs' imported but unused
(F401)
[error] 12-12: 'pdstable' imported but unused
(F401)
[error] 15-15: 'filecache.FileCache' imported but unused
(F401)
[error] 28-28: block comment should start with '# '
(E265)
[error] 30-30: continuation line over-indented for visual indent
(E127)
[error] 570-570: block comment should start with '# '
(E265)
metadata_tools/common.py
[error] 20-20: continuation line under-indented for visual indent
(E128)
[error] 23-23: block comment should start with '# '
(E265)
[error] 39-39: block comment should start with '# '
(E265)
[error] 51-51: block comment should start with '# '
(E265)
[error] 62-62: block comment should start with '# '
(E265)
[error] 78-78: block comment should start with '# '
(E265)
[error] 97-97: block comment should start with '# '
(E265)
[error] 156-156: block comment should start with '# '
(E265)
[error] 195-195: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/geometry_config.py
[error] 20-20: block comment should start with '# '
(E265)
[error] 54-54: continuation line missing indentation or outdented
(E122)
[error] 55-55: continuation line missing indentation or outdented
(E122)
[error] 55-55: block comment should start with '# '
(E265)
[error] 67-67: continuation line over-indented for visual indent
(E127)
[error] 74-74: continuation line over-indented for visual indent
(E127)
[error] 78-78: continuation line over-indented for visual indent
(E127)
[error] 88-88: continuation line over-indented for visual indent
(E127)
[error] 94-94: block comment should start with '# '
(E265)
metadata_tools/util.py
[error] 8-8: redefinition of unused 're' from line 5
(F811)
[error] 14-14: 'metadata_tools.common as com' imported but unused
(F401)
[error] 17-17: block comment should start with '# '
(E265)
[error] 29-29: local variable 'local_table_path' is assigned to but never used
(F841)
[error] 32-32: block comment should start with '# '
(E265)
[error] 109-109: block comment should start with '# '
(E265)
[error] 122-122: block comment should start with '# '
(E265)
[error] 385-385: block comment should start with '# '
(E265)
[error] 526-526: block comment should start with '# '
(E265)
[error] 538-538: block comment should start with '# '
(E265)
[error] 553-553: continuation line over-indented for visual indent
(E127)
[error] 557-557: block comment should start with '# '
(E265)
[error] 576-576: undefined name 'config'
(F821)
[error] 585-585: block comment should start with '# '
(E265)
[error] 773-773: block comment should start with '# '
(E265)
metadata_tools/defs.py
[error] 5-5: 'inspect' imported but unused
(F401)
metadata_tools/geometry_support.py
[error] 97-97: continuation line missing indentation or outdented
(E122)
[error] 123-123: continuation line unaligned for hanging indent
(E131)
[error] 214-214: continuation line over-indented for visual indent
(E127)
[error] 217-217: continuation line over-indented for visual indent
(E127)
[error] 219-219: block comment should start with '# '
(E265)
[error] 257-257: block comment should start with '# '
(E265)
[error] 284-284: continuation line over-indented for visual indent
(E127)
[error] 314-314: block comment should start with '# '
(E265)
[error] 329-329: block comment should start with '# '
(E265)
[error] 361-361: block comment should start with '# '
(E265)
[error] 433-433: block comment should start with '# '
(E265)
[error] 452-452: block comment should start with '# '
(E265)
[error] 454-454: continuation line over-indented for visual indent
(E127)
[error] 455-455: continuation line over-indented for visual indent
(E127)
[error] 510-510: continuation line under-indented for visual indent
(E128)
[error] 511-511: continuation line under-indented for visual indent
(E128)
[error] 512-512: continuation line under-indented for visual indent
(E128)
[error] 513-513: continuation line under-indented for visual indent
(E128)
[error] 514-514: continuation line under-indented for visual indent
(E128)
[error] 515-515: continuation line under-indented for visual indent
(E128)
[error] 526-526: block comment should start with '# '
(E265)
[error] 604-604: continuation line over-indented for visual indent
(E127)
[error] 605-605: continuation line over-indented for visual indent
(E127)
[error] 606-606: continuation line over-indented for visual indent
(E127)
[error] 904-904: block comment should start with '# '
(E265)
[error] 1006-1006: block comment should start with '# '
(E265)
[error] 1041-1041: block comment should start with '# '
(E265)
[error] 1071-1071: block comment should start with '# '
(E265)
[error] 1101-1101: block comment should start with '# '
(E265)
[error] 1141-1141: block comment should start with '# '
(E265)
[error] 1175-1175: continuation line over-indented for visual indent
(E127)
[error] 1246-1246: block comment should start with '# '
(E265)
[error] 1278-1278: closing bracket does not match visual indentation
(E124)
[error] 1283-1283: block comment should start with '# '
(E265)
[error] 1318-1318: continuation line missing indentation or outdented
(E122)
🪛 LanguageTool
README.md
[style] ~85-~85: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...in the summary templates as needed. 7. Edit the supplemental template to define the...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~107-~107: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ictionary in geometry_support.py. 4. Add column description(s) to the label temp...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
README.md
95-95: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🪛 OSV Scanner (2.3.1)
requirements.txt
[HIGH] 1-1: urllib3 1.26.20: urllib3 streaming API improperly handles highly compressed data
[HIGH] 1-1: urllib3 1.26.20: Decompression-bomb safeguards bypassed when following HTTP redirects (streaming API)
[HIGH] 1-1: urllib3 1.26.20: urllib3 allows an unbounded number of links in the decompression chain
[HIGH] 1-1: urllib3 1.26.20: urllib3 redirects are not disabled when retries are disabled on PoolManager instantiation
🪛 Ruff (0.14.10)
metadata_tools/label_support.py
15-15: Boolean default positional argument in function definition
(FBT002)
metadata_tools/column/COLUMNS_RING.py
95-95: Undefined name defs
(F821)
96-96: Undefined name defs
(F821)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py
26-26: Unused function argument: task_id
(ARG001)
48-48: Unpacked variable index_type is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
48-48: Unpacked variable template_dir is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py
42-42: Unused function argument: task_id
(ARG001)
61-61: Unpacked variable template_dir is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
metadata_tools/column/COLUMNS_BODY.py
54-54: Undefined name defs
(F821)
55-55: Undefined name defs
(F821)
metadata_tools/columns.py
29-29: Use of exec detected
(S102)
metadata_tools/cumulative_support.py
16-16: Missing return type annotation for private function _cat_rows
Add return type annotation: None
(ANN202)
129-129: Unpacked variable index_type is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
140-140: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
140-140: Logging statement uses %
(G002)
metadata_tools/index_support.py
29-29: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
30-30: Missing type annotation for **kwargs
(ANN003)
73-73: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
73-73: Logging statement uses %
(G002)
107-107: Boolean default positional argument in function definition
(FBT002)
479-479: Missing return type annotation for private function _create_index
Add return type annotation: None
(ANN202)
481-481: Boolean default positional argument in function definition
(FBT002)
486-486: Boolean default positional argument in function definition
(FBT002)
524-524: Loop control variable files not used within loop body
Rename unused files to _files
(B007)
567-567: Logging statement uses warn instead of warning
Convert to warning
(G010)
576-576: Boolean default positional argument in function definition
(FBT002)
metadata_tools/common.py
32-32: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
59-60: Replace yield over for loop with yield from
Replace with yield from
(UP028)
71-71: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
71-71: Logging statement uses %
(G002)
98-98: Boolean default positional argument in function definition
(FBT002)
111-111: Missing return type annotation for special method __call__
(ANN204)
111-111: Unused method argument: parser
(ARG002)
111-111: Unused method argument: option_string
(ARG002)
152-152: Class Table inherits from object
Remove object inheritance
(UP004)
157-157: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
159-159: Boolean default positional argument in function definition
(FBT002)
189-189: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
196-196: Boolean default positional argument in function definition
(FBT002)
metadata_tools/hosts/GO_0xxx/geometry_config.py
95-95: Unused function argument: observation
(ARG001)
metadata_tools/util.py
8-8: Redefinition of unused re from line 5
Remove definition: re
(F811)
29-29: Local variable local_table_path is assigned to but never used
Remove assignment to unused variable local_table_path
(F841)
576-576: Undefined name config
(F821)
753-753: Missing return type annotation for private function _ninety_percent_gap_degrees
(ANN202)
774-774: Missing return type annotation for private function _get_range_mod360
(ANN202)
metadata_tools/geometry_support.py
139-139: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
220-220: Missing return type annotation for private function _inventory
(ANN202)
234-234: Consider moving this statement to an else block
(TRY300)
244-244: Logging statement uses warn instead of warning
Convert to warning
(G010)
248-248: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
254-254: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
258-258: Missing return type annotation for private function _select_bodies
(ANN202)
316-316: Missing return type annotation for staticmethod get_backplane_key
(ANN205)
347-347: Prefer next(iter(column_descs.keys())) over single element slice
Replace with next(iter(column_descs.keys()))
(RUF015)
373-373: Missing return type annotation for private function link_null
(ANN202)
434-434: Missing return type annotation for private function _get_system
(ANN202)
454-454: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
509-509: Unpacked variable overrides is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
527-527: Missing return type annotation for private function _prep_row
(ANN202)
529-529: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
693-693: Boolean positional value in function call
(FBT003)
698-698: Boolean positional value in function call
(FBT003)
725-725: Boolean positional value in function call
(FBT003)
875-875: Missing return type annotation for private function _circle_coverage
(ANN202)
905-905: Missing return type annotation for private function _formatted_column
(ANN202)
961-961: No explicit stacklevel keyword argument found
Set stacklevel=2
(B028)
964-964: No explicit stacklevel keyword argument found
Set stacklevel=2
(B028)
1007-1007: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1007-1007: Missing type annotation for **kwargs
(ANN003)
1042-1042: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1042-1042: Missing type annotation for **kwargs
(ANN003)
1072-1072: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1072-1072: Missing type annotation for **kwargs
(ANN003)
1102-1102: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1102-1102: Missing type annotation for **kwargs
(ANN003)
1142-1142: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1142-1142: Missing type annotation for **kwargs
(ANN003)
1174-1174: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1248-1248: Missing return type annotation for staticmethod get_override
(ANN205)
1285-1285: Missing return type annotation for staticmethod get_overrides
(ANN205)
1374-1374: Boolean default positional argument in function definition
(FBT002)
1414-1414: Use format specifiers instead of percent format
(UP031)
1414-1414: Logging statement uses %
(G002)
1494-1494: Boolean default positional argument in function definition
(FBT002)
1519-1519: Unpacked variable index_type is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
1541-1541: Loop control variable files not used within loop body
Rename unused files to _files
(B007)
🪛 Shellcheck (0.11.0)
metadata_tools/hosts/GO_0xxx/gcp_geometry_startup.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 5-5: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[info] 7-7: Not following: venv/bin/activate was not specified as input (see shellcheck -x).
(SC1091)
[info] 10-10: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 10-10: Double quote to prevent globbing and word splitting.
(SC2086)
metadata_tools/hosts/GO_0xxx/gcp_index_startup.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 8-8: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 10-10: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[info] 12-12: Not following: venv/bin/activate was not specified as input (see shellcheck -x).
(SC1091)
[warning] 16-16: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🪛 YAMLlint (1.37.1)
metadata_tools/hosts/GO_0xxx/gcp_index_config.yml
[error] 3-3: too many spaces after colon
(colons)
[error] 4-4: too many spaces after colon
(colons)
[error] 5-5: too many spaces after colon
(colons)
[error] 6-6: too many spaces after colon
(colons)
[error] 7-7: too many spaces after colon
(colons)
[error] 8-8: too many spaces after colon
(colons)
[error] 9-9: too many spaces after colon
(colons)
| provider: gcp | ||
| gcp: | ||
| job_id: metadata_geometry-job | ||
| startup_script_file: gcp_geometry_startup.sh | ||
| max-instances: 5 | ||
| max-cpu: 8 | ||
| min-memory-per-cpu: 4 # GB | ||
| max-total-price-per-hour: 1.00 # USD/hour | ||
| instance-types: "n2-" | ||
| region: us-central1 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
us-central1 GCP region valid availability
💡 Result:
The us-central1 region provides these availability zones: us-central1-a, us-central1-b, us-central1-c, and us-central1-f. [1][2]
Sources:
[1] Google Cloud — Filestore: Regions and zones.
[2] Google Cloud — Geography and regions.
🌐 Web query:
GCP n2 instance family CPU memory specifications
💡 Result:
Summary — N2 machine family (Compute Engine)
- vCPU range: 2–128 vCPUs. [1]
- Memory per vCPU: configurable from 0.5 to 8 GB (i.e., 0.5 GB/vCPU for high‑CPU, 4 GB/vCPU for standard, 8 GB/vCPU for high‑memory). [1][2]
- Maximum memory supported: up to 864 GB total. [1]
- Predefined subfamilies and typical ratios:
- n2-highcpu: 0.5 GB per vCPU (e.g., n2-highcpu-2 = 2 vCPU / 2 GB, n2-highcpu-32 = 32 vCPU / 32 GB). [2]
- n2-standard: 4 GB per vCPU (e.g., n2-standard-8 = 8 vCPU / 32 GB). [2]
- n2-highmem: 8 GB per vCPU (e.g., n2-highmem-4 = 4 vCPU / 32 GB). [3]
- CPU platforms: Cascade Lake (default up to certain sizes) and Ice Lake (available for larger types / specific regions). [1]
Sources: Google Cloud docs — General-purpose machines and N2 series. [1][2][3]
🌐 Web query:
GCP n2 instance pricing us-central1 2024 2025
💡 Result:
Here are the on‑demand GCP N2 prices in us-central1 (Iowa) shown on Google’s pricing pages (Compute VM pricing) — hourly rates (and approximate monthly = hourly × 24 × 30):
- n2-standard-2 (2 vCPU, 8 GiB): $0.097118 /hr ≈ $70 /month. [1]
- n2-standard-4 (4 vCPU, 16 GiB): $0.194236 /hr ≈ $141.8 /month. [1]
Source: Google Cloud VM instance pricing (us-central1). Prices are on‑demand rates and subject to change; see the Google Cloud pricing page for other N2 sizes, discounts (sustained use, committed use), and historical billing. [1]
References
[1] Google Cloud — VM instance pricing (Compute Engine).
Fix GCP budget configuration—current cap is insufficient for the workload.
The region us-central1 and instance family n2-standard-8 (8 vCPU, 32GB RAM) are valid. However, the max-total-price-per-hour: 1.00 budget is inadequate for running 5 instances simultaneously. A single n2-standard-8 instance in us-central1 costs approximately $0.39/hour on-demand; 5 instances would cost ~$1.94/hour, exceeding your budget by nearly 100%.
To resolve, either:
- Reduce
max-instancesto 2–3 instances, or - Downgrade to smaller instance types (e.g., n2-standard-4), or
- Increase
max-total-price-per-hourto accommodate the intended workload
🤖 Prompt for AI Agents
In @metadata_tools/hosts/GO_0xxx/gcp_geometry_config.yml around lines 1 - 10,
The GCP budget cap is too low for the requested fleet: with region and instance
family set to us-central1 and instance-types "n2-" targeting n2-standard-8 (8
vCPU, 32GB) five instances will exceed max-total-price-per-hour: 1.00; fix by
either lowering max-instances to 2–3, switching instance-types to a smaller SKU
such as n2-standard-4 (adjust max-cpu and min-memory-per-cpu accordingly), or
increasing max-total-price-per-hour to cover ~0.39/hour per n2-standard-8 (so
~2.0+/hour for 5 instances); update the gcp config keys max-instances,
instance-types (and related max-cpu/min-memory-per-cpu if changing size), or
max-total-price-per-hour to reflect the chosen option.
There was a problem hiding this comment.
Actionable comments posted: 32
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
metadata_tools/column/COLUMNS_BODY.py (1)
87-87: Fix undefined variableBODIESat line 87.Line 87 references
BODIES, but this variable is not defined or imported in this file.BODIESis defined inmetadata_tools/columns.py. Either importBODIESfrom that module or usedefs.BODY_NAMESinstead (as done at line 100).metadata_tools/hosts/GO_0xxx/geometry_config.py (1)
1-6: Header comment references wrong filename.The header comment on line 2 says
# host_config.py for GLL SSIbut this file isgeometry_config.py. Update the comment to match the actual filename.🔧 Proposed fix
########################################################################################## -# host_config.py for GLL SSI +# geometry_config.py for GLL SSI # # Host-specific definitions and utilites for geometry file generation. # ##########################################################################################metadata_tools/util.py (1)
288-310: Bug:resultused before assignment whenfilespecis a string.On line 302,
resultis used but is only assigned on line 300 whenfilespecis not a string. Iffilespecis astr,resultwill be undefined, causing aNameError.🔧 Proposed fix
def expandvars(filespec): ### add to FCPath? """Expand environment variables in path. ... """ - if not isinstance(filespec, str): - result = filespec.as_posix() + if isinstance(filespec, str): + result = filespec + else: + result = filespec.as_posix() result = re.sub('://', '<<token>>', result) result = os.path.expandvars(result) result = re.sub('<<token>>', '://', result) if isinstance(filespec, str): - return result.as_posix() + return result if isinstance(filespec, FCPath): return FCPath(result) return Path(result)metadata_tools/index_support.py (1)
520-527: Unused loop variablefiles.The
filesvariable from thewalk()call is never used in the loop body. Rename it to_filesto indicate it's intentionally unused.♻️ Proposed fix
- for root, dirs, files in volume_tree.walk(): + for root, dirs, _files in volume_tree.walk():metadata_tools/geometry_support.py (1)
1544-1551: Unused loop variablefiles.The
filesvariable from thewalk()call is never used. Rename to_filesto indicate it's intentionally unused.♻️ Proposed fix
- for root, dirs, files in volume_tree.walk(): + for root, dirs, _files in volume_tree.walk():
🤖 Fix all issues with AI agents
In @metadata_tools/column/COLUMNS_BODY.py:
- Around line 56-57: Import or reference the correct body list to avoid a
NameError: replace the unresolved BODIES reference by importing BODIES from
metadata_tools.columns or use the already-available defs.BODY_NAMES where BODIES
is used; update all usages to the chosen symbol (e.g., BODIES or
defs.BODY_NAMES) so names resolve at runtime. Also clarify and make consistent
the upper-bound change for the limb fields: verify Backplane.evaluate accepts an
upper bound of 3, then either keep both tuples ("limb_altitude", defs.BODYX,
-0.01, 3, True) and ("limb_clock_angle", defs.BODYX, -0.01, 3, True) if 3 is
intended, or revert them to None (as previously commented) if no numeric limit
was desired; add a short inline comment documenting which choice was made and
why to prevent future confusion.
In @metadata_tools/columns.py:
- Line 29: The change uses file.as_posix() when calling exec(open(...).read()),
which is unnecessary because Python 3.6+ open() accepts pathlib.Path objects;
revert exec(open(file.as_posix()).read()) back to exec(open(file).read()) (or
use exec(open(str(file)).read()) if you want explicit string conversion) in
metadata_tools/columns.py so the code remains simple and clear while preserving
correct behavior when file is a Path.
In @metadata_tools/common.py:
- Around line 86-90: The post-with explicit close is redundant and wrong: remove
the standalone file.close() after the with open(task_file, "w") as file: block
so the file is closed automatically; keep the existing guard if not task_file:
return and the json.dump(task_list, file, indent=2) call inside the with block
(symbols: task_file, with open(..., "w") as file, json.dump, task_list).
- Around line 185-193: The method returns early when output_dir is falsy but
only initializes self.rows later, causing AttributeError on later write() calls;
move or add initialization of self.rows (e.g., set self.rows = [] before the "if
not output_dir: return" check) so that self.rows always exists, and keep the
existing filename/suffix logic unchanged (refer to symbols: output_dir,
self.rows, write()).
- Around line 140-142: The --labels argument is inconsistent:
gr.add_argument('--labels', '-l', type=str, default=False, ...) mixes a string
type with a boolean default; change this to either make it an optional flag or a
string option. If labels should be a flag, replace type/default with
action='store_true' (and remove type=str) so args.labels is always a bool; if it
should accept a string value, set default=None (or an empty string) and keep
type=str so args.labels is consistently a string or None. Update any code using
args.labels accordingly (e.g., checks in functions that read args.labels).
In @metadata_tools/cumulative_support.py:
- Around line 129-132: The tuple returned by
util.parse_template_name(template_name) unpacks an unused value as index_type;
change the unpack to host, _index_type, template_dir (or host, _, template_dir)
to prefix the unused variable with an underscore so linters and readers know it
is intentionally ignored — update the unpacking where
parse_template_name(template_name) is called (and adjust any related references
if present) while leaving subsequent uses of host, template_dir,
FCPath(template_name).with_suffix('.lbl'), and get_args(...) unchanged.
In @metadata_tools/defs.py:
- Line 5: The file contains an unused import "inspect"; remove the unused import
statement (the "inspect" identifier) from the top-level imports in
metadata_tools/defs.py so there are no unused imports reported and no references
remain to "inspect" in that module.
In @metadata_tools/geometry_support.py:
- Around line 1580-1593: The call to com.write_task_file(task_file) is currently
inside the volume_tree.walk loop and thus rewrites the task file each iteration;
move the write out of the loop so it runs once after all volumes are processed:
ensure the write occurs after the for root, dirs, _files in volume_tree.walk():
loop finishes, keep the call gated by the same task_list_only condition, and
adjust indentation accordingly so com.write_task_file(task_file) executes once
(refer to com.write_task_file, com, task_list_only, and the volume walk / Suite
processing block).
- Around line 1276-1282: The tuple unpacking for 'format' incorrectly expects 8
values while FORMAT_DICT entries have 10; fix by extracting the needed fields
(NULL_VALUE, VALID_MINIMUM, VALID_MAXIMUM) from the actual tuple shape instead
of the current eight-variable unpack. Modify the code around the '(_,_,_,_,_,
null_value, valid_minimum, valid_maximum) = format' line in geometry_support.py
so it either uses a starred unpack to ignore the extra elements and capture the
last three (e.g., star-unpack to get null_value, valid_minimum, valid_maximum)
or directly index into the 'format' tuple to read the last three elements, then
build the 'override' and call overrides.append(override) as before.
- Around line 509-517: The variable overrides returned from self._prep_row is
captured but unused; either incorporate it into the instance state or mark it as
intentionally ignored. Fix by either using the returned value (e.g.,
append/merge into self.overrides like the commented line self.overrides +=
overrides) or change the assignment to ignore it (rows, _ = self._prep_row(...))
and remove the commented line; update the call site referencing _prep_row and
the symbols rows, overrides, and self.overrides accordingly.
In @metadata_tools/hosts/GO_0xxx/gcp_index_config.yml:
- Around line 3-9: The YAML file has extra alignment spaces after colons on keys
like job_id, project_id, startup_script_file, service_account, max_runtime,
region, and max_instances which triggers yamllint's `colons` rule; fix it by
removing the extra spaces so each mapping is "key: value" (e.g., change "job_id:
metadata-index-job" to "job_id: metadata-index-job") or, if alignment is
preferred, update the yamllint configuration to allow colons alignment instead
of changing the file.
In @metadata_tools/hosts/GO_0xxx/gcp_index_startup.sh:
- Around line 8-16: Add robust error handling around directory changes and
critical setup steps: enable strict mode at the top (e.g., set -euo pipefail)
and guard each directory change and activation command so failures abort with a
clear message (e.g., replace plain "cd rms-metadata-tools" and the final "cd
hosts/GO_0xxx" with "cd rms-metadata-tools || { echo 'failed to cd
rms-metadata-tools'; exit 1; }" and "cd hosts/GO_0xxx || { echo 'failed to cd
hosts/GO_0xxx'; exit 1; }", and guard "source venv/bin/activate" similarly),
ensuring any failed git clone, venv activation, or pip install is detected and
stops the script.
- Line 1: Add a bash shebang as the very first line of gcp_index_startup.sh (use
/bin/bash) so the script has an explicit interpreter; ensure the shebang is the
topmost line, followed by a newline, and commit the change.
- Around line 2-5: The script gcp_index_startup.sh contains three commented
apt-get error lines (the lines starting with "# E: Could not open lock file" and
"# E: Unable to lock directory") that are leftover debug output; remove those
commented error message lines so the script only contains functional comments
and commands, leaving any meaningful header or usage comments intact.
In @metadata_tools/hosts/GO_0xxx/geometry_config.py:
- Around line 95-104: The docstring for function except_test incorrectly states
the return type as "str"; update the Returns section of the except_test
docstring to reflect the actual boolean return (e.g., "Returns: bool: True if
the observation should be an exception.") so the documented type matches the
returned value from except_test.
In @metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py:
- Line 25: Update the block comment separator lines that use "#" with no
following space (e.g., the lines currently written as
"#========================================================================================"
at the two separator occurrences) to include a space after the hash (e.g., "#
========================================================================================"),
and apply the same change to the other identical separator occurrence so all
block comments follow the "# " PEP 8 convention.
- Around line 65-67: The code is accessing the private attribute
worker._data.args; replace this with a public API from the Worker class instead:
inspect the cloud_tasks.Worker for an existing accessor (e.g., worker.args,
worker.get_args(), worker.get_parsed_args() or worker.data) and use that to pass
args and task_file into the call (replace both occurrences of
worker._data.args). If no public accessor exists, either add a small
wrapper/helper on your side that reads arguments via the supported public
interface of Worker or open an upstream change/request to expose a public getter
on Worker, then update the call sites to use that public getter rather than
worker._data.
In @metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py:
- Line 41: Fix the block comment formatting by ensuring each comment line begins
with a hash followed by a space; replace instances of lines like
"#========================================================================================"
(e.g., the block comment markers at the locations currently shown on lines 41
and 54) with "#
========================================================================================"
so they start with "# " and match the project style.
- Line 61: The unpacked variable template_dir returned from
util.parse_template_name(hconf.template_name) is unused; rename it to
_template_dir (or simply _) in the assignment (e.g., host, index_type,
_template_dir = util.parse_template_name(...)) to indicate it is intentionally
unused and silence linter warnings while leaving host and index_type unchanged.
- Around line 73-75: The code is directly referencing the Worker's private
attribute worker._data.args; replace this by using a public API instead (e.g.,
worker.args or worker.get_args()) or, if no public accessor exists, add a public
accessor on Worker (such as get_args() or a .args property) and call that here;
update the call that currently passes worker._data.args to use the new public
accessor so the module no longer depends on Worker internals (refer to the
symbols worker._data.args, task_list_only, and task_file when making the
change).
In
@metadata_tools/hosts/GO_0xxx/gs:/rms-jspitale/metadata_test/GO_0xxx/GO_0016/GO_0016_supplemental_index.lbl:
- Around line 36-37: The DESCRIPTION string for VOLUME_ID is an incomplete
sentence; update the DESCRIPTION value (the DESCRIPTION field associated with
VOLUME_ID in the label) to read: "The volume ID provides a unique identifier for
a PDS data volume." ensuring the quoted value replaces the current incomplete
text.
- Around line 87-94: The DESCRIPTION value for the LABEL (the multi-line string
containing "Galileo SSI images...") has a broken hyphenation and spacing in the
phrase "The cut out - window"; update the DESCRIPTION text so the phrase reads
consistently (e.g., "The cut out window element indicates the location and size
of this image area as defined by four numbers: starting line, starting sample,
number of lines, number of samples...") and ensure "CUT OUT WINDOW"
capitalization and spacing match PDS3 label conventions across the DESCRIPTION.
In
@metadata_tools/hosts/GO_0xxx/gs:/rms-jspitale/metadata_test/GO_0xxx/GO_0016/GO_0016_supplemental_index.tab:
- Around line 1-133: The file committed at
metadata_tools/hosts/GO_0xxx/gs:/rms-jspitale/metadata_test/GO_0xxx/GO_0016/GO_0016_supplemental_index.tab
contains an invalid GCS-style path (note "gs:/rms-jspitale" and colon) and must
be removed from the PR; delete this file from the branch, move the content into
an appropriate test fixture (e.g.,
tests/fixtures/GO_0xxx/GO_0016_supplemental_index.tab) if it’s needed, add the
generated output location to .gitignore, and update the producing code/path
configuration so generated files are written to the correct local/test directory
(not a repo path containing "gs:/...") or to actual GCS storage.
In @metadata_tools/hosts/GO_0xxx/templates/GO_0xxx_body_summary.lbl:
- Around line 33-44: The conditional uses the wrong variable name: replace
TABLE_TYPE with the defined variable index_type in the template conditional (the
$IF(TABLE_TYPE == 'CUMULATIVE') block) so it reads $IF(index_type ==
'CUMULATIVE') and likewise update any matching $ELSE/$END_IF block usage if
present to ensure the conditional evaluates the correct variable.
In @metadata_tools/hosts/GO_0xxx/tests/test_geometry.py:
- Around line 8-9: Fix the PEP8 block comment issue in test_geometry.py by
either removing the dead imports ("metadata_tools.util as util" and "import
metadata_tools.hosts.GO_0xxx.host_config as config") if they are unused, or
reformatting them to valid comments with a space after the hash (e.g. "#
metadata_tools.util as util") or restoring them as real imports if they are
required by tests; update the lines that reference these symbols to reflect the
chosen action.
In @metadata_tools/index_support.py:
- Around line 400-405: Replace the unsafe eval(result) used for format
validation with a safe literal evaluator: import ast and call
ast.literal_eval(result) inside the try block (catching ValueError and
SyntaxError) so only Python literals are accepted; update the except to log the
same warning using logger.warning('Format error for %s: %s' % (name, value))
when ast.literal_eval fails, or remove the validation entirely if it’s
unnecessary (targeting the code that currently does try: _ = eval(result) except
Exception: ... and the variables result, name, value, logger).
In @metadata_tools/label_support.py:
- Around line 14-17: The function signature's continuation lines (parameters
like system, use_global_template, table_type) are over-indented causing an E127
style issue; reformat the multi-line def so parameters align with the opening
parenthesis or follow a 4-space hanging indent convention (e.g., align
subsequent parameter lines under the first character after the opening '('),
updating the signature that contains system, use_global_template, and table_type
to use consistent indentation.
In @metadata_tools/util.py:
- Line 14: The bare import of geometry_config in metadata_tools/util.py is
fragile and can raise ModuleNotFoundError; replace the top-level plain import
with a package-aware import (use a package-relative import or an absolute
package path that resolves from the project root) so the module is resolved
reliably, ensure the package has __init__.py files as needed, and/or document
the required PYTHONPATH/runtime layout so geometry_config is importable in
standard execution contexts.
In @README.md:
- Line 95: The snippet showing key__<NAME>(label_path, label_dict) is formatted
with indentation; replace it with a fenced code block using triple backticks
(```), preferably specifying the language (e.g., ```python) before the line and
closing with ``` after, so the key__<NAME>(label_path, label_dict) line is
enclosed for clearer formatting and syntax highlighting.
In @requirements-cloud.txt:
- Around line 1-12: The requirements file lists many packages without version
constraints (e.g., numpy, rms-oops, rms-julian, rms-pdsparser, json-stream,
rms-pdstable, rms-vicar, rms-pdstemplate, fortranformat); update
requirements-cloud.txt to add minimum version pins (using >=) for each of these
entries to ensure reproducible builds—choose appropriate baseline versions (or
the current tested versions in your environment) and replace lines like "numpy"
with "numpy>=X.Y.Z", and similarly for each rms-* and other packages, leaving
already constrained packages (e.g., rms-filecache>=3.0.0, rms-pdslogger>=3.0.0,
rms-cloud-tasks>=0.1.0) unchanged unless you want to tighten them as well.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (50)
.flake8README.mdmetadata_tools/__init__.pymetadata_tools/column/COLUMNS_BODY.pymetadata_tools/column/COLUMNS_RING.pymetadata_tools/columns.pymetadata_tools/common.pymetadata_tools/cumulative_support.pymetadata_tools/defs.pymetadata_tools/geometry_support.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_cumulative.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_geometry.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_index.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.pymetadata_tools/hosts/GO_0xxx/addition_tasks.jsonmetadata_tools/hosts/GO_0xxx/gcp_geometry_config.ymlmetadata_tools/hosts/GO_0xxx/gcp_geometry_startup.shmetadata_tools/hosts/GO_0xxx/gcp_index_config.ymlmetadata_tools/hosts/GO_0xxx/gcp_index_startup.shmetadata_tools/hosts/GO_0xxx/geometry_config.pymetadata_tools/hosts/GO_0xxx/gs:/rms-jspitale/metadata_test/GO_0xxx/GO_0016/GO_0016_supplemental_index.lblmetadata_tools/hosts/GO_0xxx/gs:/rms-jspitale/metadata_test/GO_0xxx/GO_0016/GO_0016_supplemental_index.tabmetadata_tools/hosts/GO_0xxx/host_config.pymetadata_tools/hosts/GO_0xxx/index_config.pymetadata_tools/hosts/GO_0xxx/index_tasks.jsonmetadata_tools/hosts/GO_0xxx/pymetadata_tools/hosts/GO_0xxx/rms-metadata-toolsmetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_body_summary.lblmetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_ring_summary.lblmetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_sky_summary.lblmetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_supplemental_index.lblmetadata_tools/hosts/GO_0xxx/tests/test_geometry.pymetadata_tools/hosts/GO_0xxx/tests/test_index.pymetadata_tools/index_support.pymetadata_tools/label_support.pymetadata_tools/stage.shmetadata_tools/templates/body_summary_columns.lblmetadata_tools/templates/edge_on_definition.lblmetadata_tools/templates/header.lblmetadata_tools/templates/inventory.lblmetadata_tools/templates/ring_summary_columns.lblmetadata_tools/templates/ring_summary_details.lblmetadata_tools/templates/sky_summary.lblmetadata_tools/templates/sky_summary_columns.lblmetadata_tools/util.pyrequirements-cloud.txtrequirements.txttests/test_geometry.pytests/unittester_support.py
💤 Files with no reviewable changes (4)
- metadata_tools/templates/sky_summary.lbl
- metadata_tools/hosts/GO_0xxx/tests/test_index.py
- metadata_tools/stage.sh
- metadata_tools/templates/header.lbl
🧰 Additional context used
🧬 Code graph analysis (12)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index.py (1)
metadata_tools/index_support.py (1)
process_index(567-615)
metadata_tools/common.py (4)
metadata_tools/geometry_support.py (2)
write(1363-1374)create(1377-1436)metadata_tools/util.py (1)
write_txt_file(348-383)metadata_tools/index_support.py (1)
create(103-150)metadata_tools/label_support.py (1)
create(13-73)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py (4)
metadata_tools/geometry_support.py (2)
process_tables(1488-1593)get_args(1444-1485)metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py (2)
process_task(42-52)main(55-78)metadata_tools/util.py (1)
parse_template_name(87-107)metadata_tools/common.py (1)
task_source(52-60)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py (3)
metadata_tools/index_support.py (2)
process_index(567-615)get_args(447-472)metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py (2)
process_task(26-39)main(42-70)metadata_tools/util.py (1)
parse_template_name(87-107)
metadata_tools/index_support.py (6)
metadata_tools/common.py (4)
Table(152-222)init_logger(24-37)get_logger(41-43)get_common_args(98-147)metadata_tools/hosts/GO_0xxx/geometry_config.py (1)
get_volume_id(176-187)metadata_tools/util.py (5)
get_index_name(49-71)PdsTable(17-29)get_volume_glob(251-266)select_dir(32-46)parse_template_name(87-107)metadata_tools/geometry_support.py (2)
create(1377-1436)get_args(1444-1485)metadata_tools/label_support.py (1)
create(13-73)metadata_tools/cumulative_support.py (1)
get_args(96-118)
metadata_tools/hosts/GO_0xxx/GO_0xxx_cumulative.py (1)
metadata_tools/cumulative_support.py (1)
create_cumulative_indexes(121-169)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry.py (1)
metadata_tools/geometry_support.py (1)
process_tables(1488-1593)
metadata_tools/label_support.py (1)
metadata_tools/util.py (1)
get_template_name(74-84)
tests/test_geometry.py (1)
tests/unittester_support.py (1)
bounds(51-85)
metadata_tools/cumulative_support.py (3)
metadata_tools/common.py (2)
get_logger(41-43)get_common_args(98-147)metadata_tools/label_support.py (1)
create(13-73)metadata_tools/util.py (2)
parse_template_name(87-107)get_volume_glob(251-266)
metadata_tools/geometry_support.py (5)
metadata_tools/util.py (2)
convert_mission_table(538-554)get_primary(585-612)metadata_tools/hosts/GO_0xxx/geometry_config.py (2)
meshgrids(121-155)meshgrid(158-168)metadata_tools/common.py (3)
get_logger(41-43)Table(152-222)write(196-222)metadata_tools/index_support.py (2)
add(153-193)create(103-150)metadata_tools/label_support.py (1)
create(13-73)
metadata_tools/hosts/GO_0xxx/tests/test_geometry.py (1)
tests/unittester_support.py (1)
bounds(51-85)
🪛 Flake8 (7.3.0)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index.py
[error] 26-26: 'host_init' imported but unused
(F401)
metadata_tools/common.py
[error] 20-20: continuation line under-indented for visual indent
(E128)
[error] 23-23: block comment should start with '# '
(E265)
[error] 39-39: block comment should start with '# '
(E265)
[error] 51-51: block comment should start with '# '
(E265)
[error] 62-62: block comment should start with '# '
(E265)
[error] 78-78: block comment should start with '# '
(E265)
[error] 97-97: block comment should start with '# '
(E265)
[error] 156-156: block comment should start with '# '
(E265)
[error] 195-195: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py
[error] 25-25: block comment should start with '# '
(E265)
[error] 41-41: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py
[error] 41-41: block comment should start with '# '
(E265)
[error] 54-54: block comment should start with '# '
(E265)
metadata_tools/defs.py
[error] 5-5: 'inspect' imported but unused
(F401)
metadata_tools/index_support.py
[error] 24-24: block comment should start with '# '
(E265)
[error] 26-26: continuation line over-indented for visual indent
(E127)
[error] 566-566: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/GO_0xxx_cumulative.py
[error] 30-30: 'host_init' imported but unused
(F401)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry.py
[error] 44-44: 'host_init' imported but unused
(F401)
metadata_tools/label_support.py
[error] 14-14: continuation line over-indented for visual indent
(E127)
[error] 15-15: continuation line over-indented for visual indent
(E127)
[error] 16-16: continuation line over-indented for visual indent
(E127)
[error] 17-17: continuation line over-indented for visual indent
(E127)
metadata_tools/util.py
[error] 16-16: block comment should start with '# '
(E265)
[error] 28-28: local variable 'local_table_path' is assigned to but never used
(F841)
[error] 31-31: block comment should start with '# '
(E265)
[error] 109-109: block comment should start with '# '
(E265)
[error] 122-122: block comment should start with '# '
(E265)
[error] 385-385: block comment should start with '# '
(E265)
[error] 525-525: block comment should start with '# '
(E265)
[error] 537-537: block comment should start with '# '
(E265)
[error] 552-552: continuation line over-indented for visual indent
(E127)
[error] 556-556: block comment should start with '# '
(E265)
[error] 584-584: block comment should start with '# '
(E265)
[error] 773-773: block comment should start with '# '
(E265)
metadata_tools/geometry_support.py
[error] 96-96: continuation line missing indentation or outdented
(E122)
[error] 122-122: continuation line unaligned for hanging indent
(E131)
[error] 213-213: continuation line over-indented for visual indent
(E127)
[error] 216-216: continuation line over-indented for visual indent
(E127)
[error] 218-218: block comment should start with '# '
(E265)
[error] 256-256: block comment should start with '# '
(E265)
[error] 283-283: continuation line over-indented for visual indent
(E127)
[error] 313-313: block comment should start with '# '
(E265)
[error] 328-328: block comment should start with '# '
(E265)
[error] 360-360: block comment should start with '# '
(E265)
[error] 432-432: block comment should start with '# '
(E265)
[error] 451-451: block comment should start with '# '
(E265)
[error] 453-453: continuation line over-indented for visual indent
(E127)
[error] 454-454: continuation line over-indented for visual indent
(E127)
[error] 511-511: continuation line under-indented for visual indent
(E128)
[error] 512-512: continuation line under-indented for visual indent
(E128)
[error] 513-513: continuation line under-indented for visual indent
(E128)
[error] 514-514: continuation line under-indented for visual indent
(E128)
[error] 515-515: continuation line under-indented for visual indent
(E128)
[error] 516-516: continuation line under-indented for visual indent
(E128)
[error] 527-527: block comment should start with '# '
(E265)
[error] 607-607: continuation line over-indented for visual indent
(E127)
[error] 608-608: continuation line over-indented for visual indent
(E127)
[error] 609-609: continuation line over-indented for visual indent
(E127)
[error] 907-907: block comment should start with '# '
(E265)
[error] 1009-1009: block comment should start with '# '
(E265)
[error] 1044-1044: block comment should start with '# '
(E265)
[error] 1074-1074: block comment should start with '# '
(E265)
[error] 1104-1104: block comment should start with '# '
(E265)
[error] 1144-1144: block comment should start with '# '
(E265)
[error] 1178-1178: continuation line over-indented for visual indent
(E127)
[error] 1249-1249: block comment should start with '# '
(E265)
[error] 1281-1281: closing bracket does not match visual indentation
(E124)
[error] 1286-1286: block comment should start with '# '
(E265)
[error] 1321-1321: continuation line missing indentation or outdented
(E122)
metadata_tools/hosts/GO_0xxx/geometry_config.py
[error] 20-20: block comment should start with '# '
(E265)
[error] 54-54: continuation line missing indentation or outdented
(E122)
[error] 55-55: continuation line missing indentation or outdented
(E122)
[error] 55-55: block comment should start with '# '
(E265)
[error] 67-67: continuation line over-indented for visual indent
(E127)
[error] 74-74: continuation line over-indented for visual indent
(E127)
[error] 78-78: continuation line over-indented for visual indent
(E127)
[error] 88-88: continuation line over-indented for visual indent
(E127)
[error] 94-94: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/tests/test_geometry.py
[error] 8-8: block comment should start with '# '
(E265)
[error] 9-9: block comment should start with '# '
(E265)
🪛 LanguageTool
README.md
[style] ~85-~85: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...in the summary templates as needed. 7. Edit the supplemental template to define the...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~107-~107: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ictionary in geometry_support.py. 4. Add column description(s) to the label temp...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
README.md
95-95: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🪛 OSV Scanner (2.3.1)
requirements.txt
[HIGH] 1-1: urllib3 1.26.20: urllib3 streaming API improperly handles highly compressed data
[HIGH] 1-1: urllib3 1.26.20: Decompression-bomb safeguards bypassed when following HTTP redirects (streaming API)
[HIGH] 1-1: urllib3 1.26.20: urllib3 allows an unbounded number of links in the decompression chain
[HIGH] 1-1: urllib3 1.26.20: urllib3 redirects are not disabled when retries are disabled on PoolManager instantiation
🪛 Ruff (0.14.10)
metadata_tools/common.py
32-32: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
59-60: Replace yield over for loop with yield from
Replace with yield from
(UP028)
71-71: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
71-71: Logging statement uses %
(G002)
98-98: Boolean default positional argument in function definition
(FBT002)
111-111: Missing return type annotation for special method __call__
(ANN204)
111-111: Unused method argument: parser
(ARG002)
111-111: Unused method argument: option_string
(ARG002)
152-152: Class Table inherits from object
Remove object inheritance
(UP004)
157-157: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
159-159: Boolean default positional argument in function definition
(FBT002)
189-189: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
196-196: Boolean default positional argument in function definition
(FBT002)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py
48-48: Unpacked variable index_type is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
48-48: Unpacked variable template_dir is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py
61-61: Unpacked variable template_dir is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
metadata_tools/columns.py
29-29: Use of exec detected
(S102)
metadata_tools/index_support.py
25-25: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
26-26: Missing type annotation for **kwargs
(ANN003)
69-69: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
69-69: Logging statement uses %
(G002)
103-103: Boolean default positional argument in function definition
(FBT002)
394-394: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
394-394: Logging statement uses %
(G002)
398-398: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
398-398: Logging statement uses %
(G002)
402-402: Use of possibly insecure function; consider using ast.literal_eval
(S307)
403-403: Do not catch blind exception: Exception
(BLE001)
404-404: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
404-404: Logging statement uses %
(G002)
475-475: Missing return type annotation for private function _create_index
Add return type annotation: None
(ANN202)
477-477: Boolean default positional argument in function definition
(FBT002)
482-482: Boolean default positional argument in function definition
(FBT002)
520-520: Loop control variable files not used within loop body
Rename unused files to _files
(B007)
572-572: Boolean default positional argument in function definition
(FBT002)
metadata_tools/cumulative_support.py
16-16: Missing return type annotation for private function _cat_rows
Add return type annotation: None
(ANN202)
129-129: Unpacked variable index_type is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
140-140: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
140-140: Logging statement uses %
(G002)
metadata_tools/util.py
28-28: Local variable local_table_path is assigned to but never used
Remove assignment to unused variable local_table_path
(F841)
753-753: Missing return type annotation for private function _ninety_percent_gap_degrees
(ANN202)
774-774: Missing return type annotation for private function _get_range_mod360
(ANN202)
metadata_tools/geometry_support.py
138-138: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
219-219: Missing return type annotation for private function _inventory
(ANN202)
233-233: Consider moving this statement to an else block
(TRY300)
257-257: Missing return type annotation for private function _select_bodies
(ANN202)
315-315: Missing return type annotation for staticmethod get_backplane_key
(ANN205)
346-346: Prefer next(iter(column_descs.keys())) over single element slice
Replace with next(iter(column_descs.keys()))
(RUF015)
372-372: Missing return type annotation for private function link_null
(ANN202)
433-433: Missing return type annotation for private function _get_system
(ANN202)
510-510: Unpacked variable overrides is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
528-528: Missing return type annotation for private function _prep_row
(ANN202)
696-696: Boolean positional value in function call
(FBT003)
701-701: Boolean positional value in function call
(FBT003)
728-728: Boolean positional value in function call
(FBT003)
878-878: Missing return type annotation for private function _circle_coverage
(ANN202)
908-908: Missing return type annotation for private function _formatted_column
(ANN202)
964-964: No explicit stacklevel keyword argument found
Set stacklevel=2
(B028)
967-967: No explicit stacklevel keyword argument found
Set stacklevel=2
(B028)
1010-1010: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1010-1010: Missing type annotation for **kwargs
(ANN003)
1045-1045: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1045-1045: Missing type annotation for **kwargs
(ANN003)
1075-1075: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1075-1075: Missing type annotation for **kwargs
(ANN003)
1105-1105: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1105-1105: Missing type annotation for **kwargs
(ANN003)
1145-1145: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1145-1145: Missing type annotation for **kwargs
(ANN003)
1177-1177: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1251-1251: Missing return type annotation for staticmethod get_override
(ANN205)
1288-1288: Missing return type annotation for staticmethod get_overrides
(ANN205)
1377-1377: Boolean default positional argument in function definition
(FBT002)
1417-1417: Use format specifiers instead of percent format
(UP031)
1417-1417: Logging statement uses %
(G002)
1497-1497: Boolean default positional argument in function definition
(FBT002)
1522-1522: Unpacked variable index_type is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
1544-1544: Loop control variable files not used within loop body
Rename unused files to _files
(B007)
metadata_tools/hosts/GO_0xxx/geometry_config.py
95-95: Unused function argument: observation
(ARG001)
🪛 Shellcheck (0.11.0)
metadata_tools/hosts/GO_0xxx/gcp_geometry_startup.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 5-5: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[info] 7-7: Not following: venv/bin/activate was not specified as input (see shellcheck -x).
(SC1091)
[info] 10-10: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 10-10: Double quote to prevent globbing and word splitting.
(SC2086)
metadata_tools/hosts/GO_0xxx/gcp_index_startup.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 8-8: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 10-10: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[info] 12-12: Not following: venv/bin/activate was not specified as input (see shellcheck -x).
(SC1091)
[warning] 16-16: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🪛 YAMLlint (1.37.1)
metadata_tools/hosts/GO_0xxx/gcp_index_config.yml
[error] 3-3: too many spaces after colon
(colons)
[error] 4-4: too many spaces after colon
(colons)
[error] 5-5: too many spaces after colon
(colons)
[error] 6-6: too many spaces after colon
(colons)
[error] 7-7: too many spaces after colon
(colons)
[error] 8-8: too many spaces after colon
(colons)
[error] 9-9: too many spaces after colon
(colons)
- move util.get_primary to Record class75
There was a problem hiding this comment.
Actionable comments posted: 29
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry.py (1)
6-41: Update usage documentation to reflect actual argument signature.The usage examples (lines 39-41) show three positional path arguments before the optional volume argument, but the documentation (lines 16-19) only describes
input_treeandoutput_tree. Based on the relevant code snippet fromgeometry_support.py, the function expectsvolume_tree,metadata_tree, andoutput_treearguments.📝 Proposed documentation fix
Common Arguments: - input_tree File path to the top to tree containing the volume + volume_tree File path to the top to tree containing the volume files. + metadata_tree File path to the top to tree containing metadata + files. output_tree File path to the top to tree in which to place the - volume files. + generated geometry files. volume If given, only this volume is processed.metadata_tools/hosts/GO_0xxx/GO_0xxx_index.py (1)
6-24: Update usage documentation to reflect actual argument signature.Similar to the geometry script, the usage examples (lines 22-23) show three positional path arguments, but the documentation (lines 12-15) only describes
input_treeandoutput_tree. Based on the relevant code snippet fromindex_support.py, clarify the full argument signature.📝 Proposed documentation fix
Common Arguments: - input_tree File path to the top to tree containing the volume + volume_tree File path to the top to tree containing the volume files. + metadata_tree File path to the metadata tree. output_tree File path to the top to tree in which to place the - volume files. + generated index files. volume If given, only this volume is processed.metadata_tools/column/COLUMNS_BODY.py (1)
87-91: Fix undefined variableBODIES.
BODIESis not defined or imported, which will cause aNameErrorat runtime. Based on the usage pattern in lines 100 and 117 and the expandedBODY_NAMESindefs.py, this should likely bedefs.BODY_NAMES.🐛 Proposed fix
BODY_SUMMARY_DICT = {} BODY_DETAILED_DICT = {} -for body in BODIES: +for body in defs.BODY_NAMES: BODY_SUMMARY_DICT.update(util.replacement_dict(BODY_SUMMARY_COLUMNS, defs.BODYX, [body])) BODY_DETAILED_DICT.update(util.replacement_dict(BODY_DETAILED_COLUMNS, defs.BODYX, [body]))metadata_tools/util.py (1)
297-309: Bug:result.as_posix()called on string at line 306.When
filespecis astr,resultis also astrafter processing. Calling.as_posix()on a string will raise anAttributeError.🐛 Proposed fix
if isinstance(filespec, str): - return result.as_posix() + return result if isinstance(filespec, FCPath): return FCPath(result) return Path(result)
🤖 Fix all issues with AI agents
In @metadata_tools/columns.py:
- Around line 27-28: The loop opens each path from column_files and calls exec
on the file contents without closing the file; change to opening each file with
a context manager: use with open(file.as_posix(), "r") as f: and then
exec(f.read(), globals(), locals()) (or exec(f.read()) preserving the current
exec context) so the file handle is closed automatically after reading; update
the loop that references column_files and the exec call accordingly.
In @metadata_tools/common.py:
- Around line 52-60: The task_source generator uses an explicit for-loop to
yield items; simplify it by replacing the loop with a direct delegation using
"yield from" to iterate over task_list. Update the task_source function to yield
from task_list (keeping the same docstring and behavior) so the generator is
cleaner and more idiomatic while still producing the same sequence of tasks.
- Line 152: The class declaration "class Table(object):" unnecessarily inherits
from object; update the Table class definition (symbol: Table in
metadata_tools/common.py) to use Python 3 implicit inheritance by changing it to
"class Table:" and remove the redundant "(object)" token wherever this pattern
appears for similar classes in the same file.
- Around line 209-215: Replace the empty-list comparison "self.rows == []" with
the idiomatic truthiness check "not self.rows" inside the branch guarded by
labels_only so the early-return uses if not self.rows:; this change applies
where labels_only, self.rows, logger.info and util.write_txt_file are used (the
block that logs "Writing:" and calls util.write_txt_file(self.filename,
self.rows)).
- Around line 32-34: The call uses the class method FCPath.unlink(path,
missing_ok=True) which is incorrect; change to call the instance method on the
Path object (use path.unlink(missing_ok=True)) so that the unlink operation is
invoked on the Path instance created earlier (symbols: name, path, dir,
FCPath.unlink). Ensure you remove the incorrect class-style call and invoke
unlink on the path variable instead.
- Around line 63-68: Fix the typo in the add_task function docstring: change
"volune" to "volume" in the Args description (the docstring under
add_task(volume_id, index_type)) so the parameter description reads "ID of
volume to add." and ensure spelling is consistent elsewhere in that docstring.
- Around line 178-192: The constructor may leave self.template_path undefined
when template_path is falsy, causing an AttributeError in write(); ensure
self.template_path is always set by assigning self.template_path =
FCPath(template_path) when template_path is truthy and otherwise setting
self.template_path = None (i.e., initialize the attribute unconditionally), so
update the initialization around the template_path handling in the constructor
(referencing self.template_path and the write() method) to guarantee the
attribute exists even if no template_path was provided.
In @metadata_tools/geometry_support.py:
- Line 1579: The tuple returned by parse_template_name is unpacked into host,
index_type, template_dir but index_type is not used; update the unpacking in the
caller (e.g., the line using parse_template_name in geometry_support.py) to use
a throwaway name like _ (host, _, template_dir =
util.parse_template_name(template_name)) so intent is clear and linters stop
flagging the unused variable.
- Around line 964-972: The warning calls in the numeric sanitization block
(where variables number, null_value, valid_minimum, and valid_maximum are
handled) use warnings.warn(...) without a stacklevel; update the two
warnings.warn invocations ("NaN encountered" and "infinity encountered") to
include stacklevel=2 so the reported location points to the caller rather than
inside geometry_support.py; locate these calls in the function that normalizes
number and add the stacklevel=2 parameter to each warnings.warn call.
In @metadata_tools/hosts/GO_0xxx/gcp_geometry_startup.sh:
- Line 4: Update the bare cd command in gcp_geometry_startup.sh to include error
handling so the script exits on failure; replace the plain "cd" invocation (the
existing cd at the top of the script) with a guarded form that checks the
command result and calls exit 1 on failure (e.g., use cd "$HOME" || exit 1) to
match the error-handling pattern used elsewhere in the script.
- Line 10: Update the shell invocation in gcp_geometry_startup.sh to quote
environment variables to avoid word-splitting (replace unquoted $RMS_METADATA
with "$RMS_METADATA") and verify the script path for GO_0xxx_geometry_cloud.py
is correct relative to the repository root (adjust to an explicit relative path
like ./tools/... if needed); ensure both arguments passed to the python3
invocation are quoted (e.g., " $RMS_METADATA/GO_0xxx/") so paths with spaces or
special characters are handled safely.
In @metadata_tools/hosts/GO_0xxx/gcp_index_config.yml:
- Around line 3-9: The YAML uses extra spaces after colons for vertical
alignment (e.g., keys like job_id, project_id, startup_script_file,
service_account, max_runtime, region, max_instances) which trips YAMLlint;
update the document to remove the padding spaces so each mapping is "key: value"
with a single space after the colon, or alternatively add a YAMLlint rule to
allow aligned values if you prefer that style—choose one and apply it
consistently across the keys mentioned.
In @metadata_tools/hosts/GO_0xxx/gcp_index_startup.sh:
- Around line 15-16: Ensure the script checks the result of changing directory
before running the Python command: after attempting to run cd hosts/GO_0xxx
(used before invoking GO_0xxx_index_cloud.py), verify the directory change
succeeded and if not log an error and exit non‑zero so the python command does
not run from the wrong location; add a conditional that tests the cd return
status and aborts with a clear message referencing hosts/GO_0xxx and
GO_0xxx_index_cloud.py.
- Around line 2-8: The script currently contains apt-get commands documented
with captured error output and a bare cd without failure handling; remove or
convert the error-output lines into a clear comment explaining the possible
permission error, ensure apt-get runs with sudo when not root (or add a UID
check and fail with a helpful message), and add robust error handling to the
bare cd by replacing it with a guarded form (e.g., cd "$DIR" || { echo "failed
to cd to $DIR"; exit 1; }) or ensure the script uses set -e so failures abort;
update the apt-get invocations to handle failures (check exit status or use || {
echo "..."; exit 1; }) and keep messages explicit for debugging.
In @metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py:
- Around line 48-51: The unpack of util.parse_template_name currently binds
unused variables index_type and template_dir; change the unpack to use
underscore-prefixed names (e.g., _index_type, _template_dir) so the intent is
clear and linters won't flag unused variables — update the line that assigns
host, index_type, template_dir and leave the subsequent get_args call (and
variable parser) unchanged.
- Around line 26-28: The type hint uses Python's built-in any instead of the
typing type; import Any from typing and replace occurrences in the process_task
signature: change task_data: dict[str, any] to task_data: dict[str, Any] and
update the return annotation from tuple[bool, any] to tuple[bool, Any]; ensure
the module has "from typing import Any" (or add Any to an existing typing
import).
- Around line 65-67: The code accesses the documented-but-privately-named
attribute worker._data.args; change the public surface instead of relying on a
private name by adding a public accessor on the Worker type (e.g., a property
Worker.args or method Worker.get_args() that returns WorkerData.args) and update
this call site to use worker.args (or worker.get_args()) and remove direct
references to worker._data; if you cannot modify the library, wrap the access in
a single helper (e.g., get_worker_args(worker)) and add a TODO/issue referencing
rms-cloud-tasks to request an official public accessor for parsed args
(Worker.args or Worker.get_args()) so future code stops using the
leading-underscore attribute.
In @metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py:
- Around line 41-52: The block comment separator above process_task uses a long
line of '=' characters; change it to a PEP 8 friendly commented line (e.g.
prefix with "# " and use a shorter, readable separator like "# ----" or convert
to a normal brief comment) so the top-of-function comment for process_task
follows the same formatting style used elsewhere (e.g., match separators used in
GO_0xxx_geometry_cloud.py) and keeps static analysis happy.
In @metadata_tools/hosts/GO_0xxx/templates/GO_0xxx_body_summary.lbl:
- Around line 40-44: The conditional uses TABLE_TYPE but should check
index_type; replace the IF guard that reads "TABLE_TYPE == 'CUMULATIVE'" with a
check against index_type (i.e., "index_type == 'CUMULATIVE'") so the cumulative
description displays when index_type is set to 'CUMULATIVE' (index_type is
assigned from volume_id earlier); make the same change in the analogous template
GO_0xxx_ring_summary.lbl where the IF still references TABLE_TYPE.
In @metadata_tools/hosts/GO_0xxx/tests/test_geometry.py:
- Around line 8-9: The two commented import lines ("#metadata_tools.util as
util" and "#import metadata_tools.hosts.GO_0xxx.host_config as config") violate
PEP8 by lacking a space after '#'; update each commented line to include a
single space after the hash (e.g., "# metadata_tools.util as util" and "# import
metadata_tools.hosts.GO_0xxx.host_config as config") so the comments conform to
style guidelines.
In @metadata_tools/index_support.py:
- Around line 400-405: The use of eval(result) in the validation block is
insecure; replace it with ast.literal_eval to safely attempt parsing literals:
import ast at top, call ast.literal_eval(result) inside the try in the same
validation section, and catch (ValueError, SyntaxError) instead of Exception (or
keep Exception if broader handling is needed), updating the logger.warning to
include the exception details and the name/value context so failures are
informative; ensure the symbol references are the existing try/except validation
around result/name/value in metadata_tools.index_support.py.
In @metadata_tools/templates/inventory.lbl:
- Line 16: The $INCLUDE("host_defs.lbl") in inventory.lbl cannot be resolved
because the template is processed with use_global_template=True and the
referenced host_defs.lbl lives in the hosts templates directory; fix by either
placing host_defs.lbl into the global templates directory, changing the $INCLUDE
to a correct relative path that points to the host_defs.lbl file in the hosts
templates directory, or removing the $INCLUDE if that file is not required by
the inventory template.
In @metadata_tools/util.py:
- Around line 107-118: Remove the stale inline comment on the pm function (the
"### move to utilities, why is this not in numpy?" remark) since pm already
lives in the utilities module; simply delete that comment and, if needed,
tighten the pm docstring to be concise and accurate while leaving the function
implementation (def pm(x): return np.array([x, -x])) unchanged.
- Around line 16-28: Remove the unused variable local_table_path: if the intent
is to force retrieval of the .tab file as a side effect, call
label_path.with_suffix('.tab').retrieve() without storing the result (or assign
to a deliberately ignored name like _local_table_path) and add a one-line
comment explaining the side-effect; otherwise simply delete the retrieval call.
Update the PdsTable function (symbol: PdsTable) accordingly so no unused
variable remains.
- Around line 512-522: The sclk_to_ticks function currently hardcodes spacecraft
ID -77; make the spacecraft ID configurable by adding a new parameter (e.g.,
spacecraft_id) with a default value of -77, update the function signature and
docstring to document the new parameter and default, and replace the literal -77
in the cspyce.sctiks_alias call with the spacecraft_id parameter; ensure any
internal or external callers are updated if they need a different ID or leave
the default unchanged.
In @requirements-cloud.txt:
- Around line 1-12: Update the rms-pdstemplate requirement in
requirements-cloud.txt from rms-pdstemplate>=2.3.1 to rms-pdstemplate>=2.3.0
(the published latest); while here, add sensible upper bounds for critical
packages to prevent accidental breaking upgrades (e.g., change numpy>=2.3.0 to
numpy>=2.3.0,<3.0.0 and apply similar <major_version caps to other key packages
as appropriate).
In @requirements.txt:
- Around line 1-6: requirements.txt currently mixes runtime and dev deps and
leaves pydantic unpinned; update pydantic to a safe version range (e.g.,
pydantic>=2.0,<3.0) and split dev/test/docs tools (IPython, coverage, flake8,
pytest, sphinx, etc.) into a new requirements-dev.txt while keeping runtime deps
(requirements-cloud.txt and pydantic) in requirements.txt; also update any
CI/README references to install from requirements-dev.txt for development and
keep production installs using requirements.txt.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (47)
.flake8README.mdmetadata_tools/__init__.pymetadata_tools/column/COLUMNS_BODY.pymetadata_tools/column/COLUMNS_RING.pymetadata_tools/columns.pymetadata_tools/common.pymetadata_tools/cumulative_support.pymetadata_tools/defs.pymetadata_tools/geometry_support.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_cumulative.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_geometry.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_index.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.pymetadata_tools/hosts/GO_0xxx/addition_tasks.jsonmetadata_tools/hosts/GO_0xxx/gcp_geometry_config.ymlmetadata_tools/hosts/GO_0xxx/gcp_geometry_startup.shmetadata_tools/hosts/GO_0xxx/gcp_index_config.ymlmetadata_tools/hosts/GO_0xxx/gcp_index_startup.shmetadata_tools/hosts/GO_0xxx/geometry_config.pymetadata_tools/hosts/GO_0xxx/host_config.pymetadata_tools/hosts/GO_0xxx/index_config.pymetadata_tools/hosts/GO_0xxx/index_tasks.jsonmetadata_tools/hosts/GO_0xxx/rms-metadata-toolsmetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_body_summary.lblmetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_ring_summary.lblmetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_sky_summary.lblmetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_supplemental_index.lblmetadata_tools/hosts/GO_0xxx/tests/test_geometry.pymetadata_tools/hosts/GO_0xxx/tests/test_index.pymetadata_tools/index_support.pymetadata_tools/label_support.pymetadata_tools/stage.shmetadata_tools/templates/body_summary_columns.lblmetadata_tools/templates/edge_on_definition.lblmetadata_tools/templates/header.lblmetadata_tools/templates/inventory.lblmetadata_tools/templates/ring_summary_columns.lblmetadata_tools/templates/ring_summary_details.lblmetadata_tools/templates/sky_summary.lblmetadata_tools/templates/sky_summary_columns.lblmetadata_tools/util.pyrequirements-cloud.txtrequirements.txttests/test_geometry.pytests/unittester_support.py
💤 Files with no reviewable changes (4)
- metadata_tools/stage.sh
- metadata_tools/templates/sky_summary.lbl
- metadata_tools/templates/header.lbl
- metadata_tools/hosts/GO_0xxx/tests/test_index.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-07T23:57:15.800Z
Learnt from: jnspitale
Repo: SETI/rms-metadata-tools PR: 97
File: metadata_tools/columns.py:29-29
Timestamp: 2026-01-07T23:57:15.800Z
Learning: When using FCPath objects from rms-filecache with Python's built-in open(), convert the FCPath to a string using .as_posix() before passing it to open. FCPath does not implement the __fspath__ protocol, so use path_str = fcpath.as_posix() and then open(path_str) (or open(path_str, mode)) to ensure compatibility.
Applied to files:
metadata_tools/hosts/GO_0xxx/GO_0xxx_cumulative.pytests/test_geometry.pymetadata_tools/column/COLUMNS_RING.pymetadata_tools/hosts/GO_0xxx/tests/test_geometry.pymetadata_tools/cumulative_support.pytests/unittester_support.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.pymetadata_tools/common.pymetadata_tools/hosts/GO_0xxx/host_config.pymetadata_tools/defs.pymetadata_tools/label_support.pymetadata_tools/__init__.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.pymetadata_tools/columns.pymetadata_tools/hosts/GO_0xxx/geometry_config.pymetadata_tools/column/COLUMNS_BODY.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_index.pymetadata_tools/hosts/GO_0xxx/index_config.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_geometry.pymetadata_tools/index_support.pymetadata_tools/util.pymetadata_tools/geometry_support.py
🧬 Code graph analysis (11)
tests/test_geometry.py (1)
tests/unittester_support.py (1)
bounds(51-85)
metadata_tools/hosts/GO_0xxx/tests/test_geometry.py (1)
tests/unittester_support.py (1)
bounds(51-85)
metadata_tools/cumulative_support.py (3)
metadata_tools/common.py (2)
get_logger(41-43)get_common_args(98-147)metadata_tools/label_support.py (1)
create(13-73)metadata_tools/util.py (2)
parse_template_name(85-105)get_volume_glob(249-264)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py (4)
metadata_tools/index_support.py (2)
process_index(567-615)get_args(447-472)metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py (2)
process_task(26-39)main(42-70)metadata_tools/util.py (1)
parse_template_name(85-105)metadata_tools/common.py (1)
task_source(52-60)
metadata_tools/common.py (4)
metadata_tools/geometry_support.py (2)
write(1420-1431)create(1434-1493)metadata_tools/util.py (1)
write_txt_file(347-382)metadata_tools/index_support.py (1)
create(103-150)metadata_tools/label_support.py (1)
create(13-73)
metadata_tools/label_support.py (3)
metadata_tools/geometry_support.py (1)
create(1434-1493)metadata_tools/index_support.py (1)
create(103-150)metadata_tools/util.py (1)
get_template_name(72-82)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py (3)
metadata_tools/geometry_support.py (1)
process_tables(1545-1650)metadata_tools/util.py (1)
parse_template_name(85-105)metadata_tools/common.py (1)
task_source(52-60)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index.py (1)
metadata_tools/index_support.py (1)
process_index(567-615)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry.py (2)
metadata_tools/geometry_support.py (1)
process_tables(1545-1650)tests/unittester_support.py (1)
exclude(30-48)
metadata_tools/index_support.py (4)
metadata_tools/common.py (4)
Table(152-222)init_logger(24-37)get_logger(41-43)get_common_args(98-147)metadata_tools/util.py (3)
get_index_name(47-69)PdsTable(16-28)parse_template_name(85-105)metadata_tools/geometry_support.py (2)
create(1434-1493)get_args(1501-1542)metadata_tools/cumulative_support.py (1)
get_args(96-118)
metadata_tools/util.py (1)
metadata_tools/common.py (1)
write(196-222)
🪛 Flake8 (7.3.0)
metadata_tools/hosts/GO_0xxx/GO_0xxx_cumulative.py
[error] 30-30: 'host_init' imported but unused
(F401)
metadata_tools/hosts/GO_0xxx/tests/test_geometry.py
[error] 8-8: block comment should start with '# '
(E265)
[error] 9-9: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py
[error] 41-41: block comment should start with '# '
(E265)
[error] 54-54: block comment should start with '# '
(E265)
metadata_tools/common.py
[error] 20-20: continuation line under-indented for visual indent
(E128)
[error] 23-23: block comment should start with '# '
(E265)
[error] 39-39: block comment should start with '# '
(E265)
[error] 51-51: block comment should start with '# '
(E265)
[error] 62-62: block comment should start with '# '
(E265)
[error] 78-78: block comment should start with '# '
(E265)
[error] 97-97: block comment should start with '# '
(E265)
[error] 156-156: block comment should start with '# '
(E265)
[error] 195-195: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py
[error] 25-25: block comment should start with '# '
(E265)
[error] 41-41: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/geometry_config.py
[error] 20-20: block comment should start with '# '
(E265)
[error] 54-54: continuation line missing indentation or outdented
(E122)
[error] 55-55: continuation line missing indentation or outdented
(E122)
[error] 55-55: block comment should start with '# '
(E265)
[error] 67-67: continuation line over-indented for visual indent
(E127)
[error] 74-74: continuation line over-indented for visual indent
(E127)
[error] 78-78: continuation line over-indented for visual indent
(E127)
[error] 88-88: continuation line over-indented for visual indent
(E127)
[error] 94-94: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index.py
[error] 26-26: 'host_init' imported but unused
(F401)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry.py
[error] 44-44: 'host_init' imported but unused
(F401)
metadata_tools/index_support.py
[error] 24-24: block comment should start with '# '
(E265)
[error] 26-26: continuation line over-indented for visual indent
(E127)
[error] 566-566: block comment should start with '# '
(E265)
metadata_tools/util.py
[error] 15-15: block comment should start with '# '
(E265)
[error] 27-27: local variable 'local_table_path' is assigned to but never used
(F841)
[error] 30-30: block comment should start with '# '
(E265)
[error] 107-107: block comment should start with '# '
(E265)
[error] 120-120: block comment should start with '# '
(E265)
[error] 384-384: block comment should start with '# '
(E265)
[error] 524-524: block comment should start with '# '
(E265)
[error] 536-536: block comment should start with '# '
(E265)
[error] 551-551: continuation line over-indented for visual indent
(E127)
[error] 714-714: block comment should start with '# '
(E265)
metadata_tools/geometry_support.py
[error] 97-97: continuation line missing indentation or outdented
(E122)
[error] 123-123: continuation line unaligned for hanging indent
(E131)
[error] 214-214: continuation line over-indented for visual indent
(E127)
[error] 217-217: continuation line over-indented for visual indent
(E127)
[error] 219-219: block comment should start with '# '
(E265)
[error] 257-257: block comment should start with '# '
(E265)
[error] 284-284: continuation line over-indented for visual indent
(E127)
[error] 314-314: block comment should start with '# '
(E265)
[error] 329-329: block comment should start with '# '
(E265)
[error] 361-361: block comment should start with '# '
(E265)
[error] 433-433: block comment should start with '# '
(E265)
[error] 452-452: block comment should start with '# '
(E265)
[error] 454-454: continuation line over-indented for visual indent
(E127)
[error] 455-455: continuation line over-indented for visual indent
(E127)
[error] 512-512: continuation line under-indented for visual indent
(E128)
[error] 513-513: continuation line under-indented for visual indent
(E128)
[error] 514-514: continuation line under-indented for visual indent
(E128)
[error] 515-515: continuation line under-indented for visual indent
(E128)
[error] 516-516: continuation line under-indented for visual indent
(E128)
[error] 517-517: continuation line under-indented for visual indent
(E128)
[error] 528-528: block comment should start with '# '
(E265)
[error] 608-608: continuation line over-indented for visual indent
(E127)
[error] 609-609: continuation line over-indented for visual indent
(E127)
[error] 610-610: continuation line over-indented for visual indent
(E127)
[error] 908-908: block comment should start with '# '
(E265)
[error] 1003-1003: block comment should start with '# '
(E265)
[error] 1030-1030: block comment should start with '# '
(E265)
[error] 1066-1066: block comment should start with '# '
(E265)
[error] 1101-1101: block comment should start with '# '
(E265)
[error] 1131-1131: block comment should start with '# '
(E265)
[error] 1161-1161: block comment should start with '# '
(E265)
[error] 1201-1201: block comment should start with '# '
(E265)
[error] 1235-1235: continuation line over-indented for visual indent
(E127)
[error] 1306-1306: block comment should start with '# '
(E265)
[error] 1338-1338: closing bracket does not match visual indentation
(E124)
[error] 1343-1343: block comment should start with '# '
(E265)
[error] 1378-1378: continuation line missing indentation or outdented
(E122)
🪛 LanguageTool
README.md
[style] ~85-~85: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...in the summary templates as needed. 7. Edit the supplemental template to define the...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~109-~109: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ictionary in geometry_support.py. 4. Add column description(s) to the label temp...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 Ruff (0.14.10)
metadata_tools/cumulative_support.py
16-16: Missing return type annotation for private function _cat_rows
Add return type annotation: None
(ANN202)
140-140: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
140-140: Logging statement uses %
(G002)
metadata_tools/common.py
32-32: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
59-60: Replace yield over for loop with yield from
Replace with yield from
(UP028)
71-71: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
71-71: Logging statement uses %
(G002)
98-98: Boolean default positional argument in function definition
(FBT002)
111-111: Missing return type annotation for special method __call__
(ANN204)
111-111: Unused method argument: parser
(ARG002)
111-111: Unused method argument: option_string
(ARG002)
152-152: Class Table inherits from object
Remove object inheritance
(UP004)
157-157: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
159-159: Boolean default positional argument in function definition
(FBT002)
190-190: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
196-196: Boolean default positional argument in function definition
(FBT002)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py
48-48: Unpacked variable index_type is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
48-48: Unpacked variable template_dir is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
metadata_tools/columns.py
28-28: Use of exec detected
(S102)
metadata_tools/hosts/GO_0xxx/geometry_config.py
95-95: Unused function argument: observation
(ARG001)
metadata_tools/index_support.py
25-25: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
26-26: Missing type annotation for **kwargs
(ANN003)
69-69: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
69-69: Logging statement uses %
(G002)
103-103: Boolean default positional argument in function definition
(FBT002)
394-394: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
394-394: Logging statement uses %
(G002)
398-398: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
398-398: Logging statement uses %
(G002)
402-402: Use of possibly insecure function; consider using ast.literal_eval
(S307)
403-403: Do not catch blind exception: Exception
(BLE001)
404-404: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
404-404: Logging statement uses %
(G002)
475-475: Missing return type annotation for private function _create_index
Add return type annotation: None
(ANN202)
477-477: Boolean default positional argument in function definition
(FBT002)
482-482: Boolean default positional argument in function definition
(FBT002)
572-572: Boolean default positional argument in function definition
(FBT002)
metadata_tools/util.py
27-27: Local variable local_table_path is assigned to but never used
Remove assignment to unused variable local_table_path
(F841)
694-694: Missing return type annotation for private function _ninety_percent_gap_degrees
(ANN202)
715-715: Missing return type annotation for private function _get_range_mod360
(ANN202)
metadata_tools/geometry_support.py
139-139: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
220-220: Missing return type annotation for private function _inventory
(ANN202)
234-234: Consider moving this statement to an else block
(TRY300)
258-258: Missing return type annotation for private function _select_bodies
(ANN202)
316-316: Missing return type annotation for staticmethod get_backplane_key
(ANN205)
347-347: Prefer next(iter(column_descs.keys())) over single element slice
Replace with next(iter(column_descs.keys()))
(RUF015)
373-373: Missing return type annotation for private function link_null
(ANN202)
434-434: Missing return type annotation for private function _get_system
(ANN202)
511-511: Unpacked variable overrides is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
529-529: Missing return type annotation for private function _prep_row
(ANN202)
697-697: Boolean positional value in function call
(FBT003)
702-702: Boolean positional value in function call
(FBT003)
729-729: Boolean positional value in function call
(FBT003)
879-879: Missing return type annotation for private function _circle_coverage
(ANN202)
909-909: Missing return type annotation for private function _formatted_column
(ANN202)
965-965: No explicit stacklevel keyword argument found
Set stacklevel=2
(B028)
968-968: No explicit stacklevel keyword argument found
Set stacklevel=2
(B028)
1004-1004: Missing return type annotation for private function _obs_excluded
(ANN202)
1031-1031: Missing return type annotation for private function _get_primary
(ANN202)
1067-1067: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1067-1067: Missing type annotation for **kwargs
(ANN003)
1102-1102: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1102-1102: Missing type annotation for **kwargs
(ANN003)
1132-1132: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1132-1132: Missing type annotation for **kwargs
(ANN003)
1162-1162: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1162-1162: Missing type annotation for **kwargs
(ANN003)
1202-1202: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1202-1202: Missing type annotation for **kwargs
(ANN003)
1234-1234: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1308-1308: Missing return type annotation for staticmethod get_override
(ANN205)
1345-1345: Missing return type annotation for staticmethod get_overrides
(ANN205)
1434-1434: Boolean default positional argument in function definition
(FBT002)
1474-1474: Use format specifiers instead of percent format
(UP031)
1474-1474: Logging statement uses %
(G002)
1554-1554: Boolean default positional argument in function definition
(FBT002)
1579-1579: Unpacked variable index_type is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🪛 Shellcheck (0.11.0)
metadata_tools/hosts/GO_0xxx/gcp_index_startup.sh
[warning] 9-9: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[info] 13-13: Not following: venv/bin/activate was not specified as input (see shellcheck -x).
(SC1091)
[warning] 15-15: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
metadata_tools/hosts/GO_0xxx/gcp_geometry_startup.sh
[warning] 4-4: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[info] 8-8: Not following: venv/bin/activate was not specified as input (see shellcheck -x).
(SC1091)
[info] 10-10: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 10-10: Double quote to prevent globbing and word splitting.
(SC2086)
🪛 YAMLlint (1.37.1)
metadata_tools/hosts/GO_0xxx/gcp_index_config.yml
[error] 3-3: too many spaces after colon
(colons)
[error] 4-4: too many spaces after colon
(colons)
[error] 5-5: too many spaces after colon
(colons)
[error] 6-6: too many spaces after colon
(colons)
[error] 7-7: too many spaces after colon
(colons)
[error] 8-8: too many spaces after colon
(colons)
[error] 9-9: too many spaces after colon
(colons)
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
metadata_tools/util.py (1)
297-309: Bug:strobject has noas_posix()method.On line 306, when
filespecis astr, the code attempts to callresult.as_posix(), butresultis already a string at this point. This will raise anAttributeError.🐛 Proposed fix
if isinstance(filespec, str): - return result.as_posix() + return result if isinstance(filespec, FCPath): return FCPath(result) return Path(result)metadata_tools/geometry_support.py (1)
1272-1276: Check for supplemental index handling when no files found.When
len(index_filenames) == 0, the method returns early without any logging or error. Consider adding a warning log to help diagnose cases where the expected index file is missing.♻️ Proposed fix
if len(index_filenames) == 0: + logger = com.get_logger() + logger.warning('No index files found matching %s in %s.' % (self.index_glob, self.metadata_dir)) return
🤖 Fix all issues with AI agents
In `@metadata_tools/column/COLUMNS_BODY.py`:
- Around line 56-59: The tuples for "limb_altitude" and "limb_clock_angle" now
pass 3 as the upper-bound parameter to Backplane.evaluate(); add a concise
inline comment next to each tuple (near the entries for limb_altitude and
limb_clock_angle) that explains what the integer 3 means (e.g., "3 = max
altitude in radii" or "3 = altitude limit in km" — use the correct domain unit),
and ensure the comment references that this value is fed into
Backplane.evaluate() so future readers understand the semantic meaning and units
of the bound.
In `@metadata_tools/common.py`:
- Around line 32-35: The unlink call is incorrect: remove the extra argument so
the Path object calls unlink without passing itself; change the call on the
variable path from path.unlink(path, missing_ok=True) to call unlink with only
the missing_ok kwarg (path.unlink(missing_ok=True)) so the file is removed
safely, leaving the subsequent _LOGGER.add_handler(pdslogger.file_handler(path,
level='warning')) unchanged.
In `@metadata_tools/geometry_support.py`:
- Line 347: Replace the inefficient list(...) indexing when extracting the first
key from column_descs with an iterator-based approach: instead of using
list(column_descs.keys())[0] to get the first key, use
next(iter(column_descs.keys())) so the code that assigns column_descs (the line
currently doing column_descs = column_descs[list(column_descs.keys())[0]])
becomes iterator-based; update that assignment to use next(iter(...)) to avoid
creating an intermediate list and improve efficiency while preserving the same
behavior.
- Around line 444-450: The _get_system function inconsistently returns str or []
— change it to return None when the body key is missing instead of an empty
list; locate the logic that checks oops.Body.BODY_REGISTRY and replace the else
branch to return None, keeping the existing returns of parent (string) and body
(string) for non-SUN parents and SUN cases respectively, and update all callers
(e.g., places mentioned around line 303 and 676) to explicitly handle a None
return value before using the result.
- Around line 598-620: In the tiles-is-tuple branch of _prep_row you initialize
overrides but never accumulate overrides returned by recursive calls; update the
loop that processes each tile so that you capture the tuple returned by
self._prep_row (e.g. new_rows, new_overrides = self._prep_row(...)), extend rows
with new_rows and extend overrides with new_overrides, and adjust local_index as
before; finally return (rows, overrides) instead of discarding overrides.
In `@metadata_tools/hosts/GO_0xxx/gcp_geometry_config.yml`:
- Around line 1-13: Uncomment and configure budget-limiting keys in the gcp
section (e.g., max-cpu, min-memory-per-cpu, max-total-price-per-hour,
instance-types) or add a clear comment explaining why they are intentionally
disabled; specifically update the gcp block where max-instances is defined so
max-cpu and min-memory-per-cpu are set to reasonable caps and
max-total-price-per-hour and instance-types are populated to limit instance size
and hourly cost, or add a documented rationale next to those keys explaining why
cost constraints are not applied.
- Around line 3-9: The YAML has excessive aligned spaces after colons for keys
like job_id, project_id, startup_script_file, service_account, max_runtime,
region, and max-instances; update the file to use standard single-space
formatting (e.g., "job_id: metadata_geometry-job") by removing the extra padding
so each key uses one space after the colon and no trailing alignment spacing.
In `@metadata_tools/hosts/GO_0xxx/gcp_geometry_startup.sh`:
- Line 5: The git clone invocation in gcp_geometry_startup.sh currently ignores
failures; update the script to detect a failed git clone and abort with a clear
error message and non-zero exit status (either enable strict failure handling at
the top of the script or check the exit status immediately after the git clone
command and print a descriptive error to stderr, then exit). Reference the git
clone command in gcp_geometry_startup.sh and ensure the error message includes
the fact that cloning the repository failed so subsequent steps do not run on a
missing checkout.
In `@metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py`:
- Around line 43-44: The type hint uses Any but Any isn't imported, causing a
NameError; update the imports in GO_0xxx_index_cloud.py to include Any (e.g.,
add Any to the existing typing import) so the signature "task_data: dict[str,
Any]" and any other uses of Any resolve correctly; ensure the import is added
near other typing imports where WorkerData/related types are referenced.
In `@metadata_tools/index_support.py`:
- Around line 600-602: Remove the leftover commented debug block that checks
"task_list_only" and prints "testing" with "args.volumes" — specifically delete
the commented lines "# if not task_list_only:", "# print('testing',
args.volumes)", and "# return" in metadata_tools/index_support.py so no
debug scaffolding remains; ensure no dependent logic relies on that commented
branch and run tests to confirm behavior unchanged.
- Around line 558-560: The call to com.write_task_file(task_file) (guarded by
task_list_only) is currently inside the volume_tree.walk() loop and causes the
task file to be written repeatedly before all volumes are collected; move the
entire write operation (and the subsequent logger.close() if present) outside
and after the for root, dirs, _files in volume_tree.walk(): loop so that
write_task_file(task_file) is executed once after volume collection is complete,
keeping the task_list_only conditional unchanged.
In `@metadata_tools/label_support.py`:
- Line 43: Validate and normalize host_template_path before accessing .parent:
ensure host_template_path is not None and is converted to an FCPath-like object
(consistent with how filepath is handled) so you don't call .parent on an
unexpected type; update the code that computes host_template_dir (referencing
host_template_path and host_template_dir) to wrap/validate host_template_path
(e.g., via FCPath or a type check and clear error) and then read .parent only
after successful normalization.
In `@metadata_tools/util.py`:
- Around line 513-524: Remove the leftover debug injection inside sclk_to_ticks
and replace it with a proper cspyce conversion call: delete the "from IPython
import embed; print('+++++++++++++'); embed()" and the malformed "cspyce."
fragment, then call the appropriate cspyce function (e.g., cspyce.sclk2et or
cspyce.sclk2ticks depending on your API) using the sc and sclk parameters and
return the integer tick/ET value so sclk_to_ticks returns a valid int; ensure
you reference the sclk_to_ticks function and cspyce namespace when making this
fix.
- Around line 527-536: The docstring for get_observation_id is incorrect: it
mentions a parameter observation_id and a return type of None even though the
function accepts observation and returns a string; update the docstring to
document the actual parameter name "observation" (describe its expected
type/structure) and change the Returns section to reflect that a string (the
observation ID) is returned so the docstring matches the function behavior.
♻️ Duplicate comments (14)
metadata_tools/common.py (2)
150-152: Remove unnecessary parentheses from class definition.In Python 3, classes inherit from
objectby default, soclass Table():can be simplified.♻️ Proposed fix
-class Table(): +class Table: """Class describing a single table for a single volume. """
138-140:--labelsargument type/default mismatch.Using
type=boolwith argparse doesn't work as expected. Argparse'stypeparameter is a callable that converts the string argument, butbool("False")returnsTrue(non-empty string). For boolean flags, useaction='store_true'instead.🐛 Proposed fix
- gr.add_argument('--labels', '-l', type=bool, metavar='labels', - default=False, - help='''If given, labels are generated for existing files.''') + gr.add_argument('--labels', '-l', action='store_true', + help='''If given, labels are generated for existing files.''')metadata_tools/columns.py (1)
26-30: FCPath remote file handling is incorrect.Using
open(file.as_posix())won't work for remote FCPath objects (e.g.,gs://,s3://,http://). FCPath provides its own I/O methods that handle remote access.As noted in the previous review discussion, the correct approach is:
🐛 Proposed fix
column_files = list(defs.COLUMN_DIR.glob('COLUMNS_*.py')) for file in column_files: -# exec(open(file.as_posix()).read()) - with open(file.as_posix()) as f: - exec(f.read()) + exec(file.read_text())Also, the
list()wrapper aroundglob()is unnecessary when iterating directly.metadata_tools/column/COLUMNS_BODY.py (1)
87-91:BODIESsymbol provided viaexec()injection.As previously discussed, this module is loaded via
exec()incolumns.pywhereBODIESis defined in the execution scope. Static analysis tools flag this as undefined, but the runtime behavior is correct.metadata_tools/geometry_support.py (5)
454-456: Mutable default argument fixed correctly.The
tiles=[]mutable default has been correctly replaced withtiles=Noneand initialized inside the function. This addresses the earlier B006 lint issue.Also applies to: 502-503
724-724: Tuple unpacking now correctly handles 10-element FORMAT_DICT entries.The unpacking has been fixed to
(_,_,_,_,_, null_value, valid_minimum, valid_maximum, _, _) = format, correctly matching the 10-element tuples in FORMAT_DICT.
244-244: Logging methods updated correctly.
logger.warn()has been replaced withlogger.warning()(line 244), andlogger.exception()is now used for unexpected errors (lines 248, 254), which automatically captures traceback information.Also applies to: 248-248
965-968:stacklevel=2added to warnings.The
warnings.warn()calls now includestacklevel=2to correctly report the caller's location rather than the internal function.
1579-1579: Unused variable_index_typefromparse_template_name.The variable has been correctly prefixed with underscore to indicate it's intentionally unused.
metadata_tools/hosts/GO_0xxx/gcp_geometry_startup.sh (2)
1-6: Shebang and error handling improvements applied.The script now includes the proper shebang (
#!/bin/bash) and|| exit 1guards oncdcommands, addressing prior review feedback.
10-12: Script path must be updated to reflect working directory context.After
cd rms-metadata-tools(line 6), the working directory is the repository root. The script invocation at line 10 usespython3 GO_0xxx_geometry_cloud.pywithout a path, but the file is located atmetadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py. This will cause a runtime failure.Additionally, consider parameterizing the GCS bucket paths using environment variables for better flexibility across environments.
🐛 Proposed fix for script path
-python3 GO_0xxx_geometry_cloud.py gs://rms-metadata-jspitale/metadata_test/GO_0xxx/ \ +python3 metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py \ + gs://rms-metadata-jspitale/metadata_test/GO_0xxx/ \ gs://rms-node-holdings/pds3-holdings/metadata/GO_0xxx/ \ gs://rms-metadata-jspitale/metadata_test/GO_0xxx/metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py (2)
65-67: Private attribute access pattern persists.The access to
worker._data.argswas flagged in previous reviews. Per those discussions, this is the documented interface forrms-cloud-tasks, though the naming convention suggests privacy. Consider wrapping this in a helper function with a TODO to request a public accessor from the library maintainers.
27-28: MissingAnyimport will causeNameErrorat runtime.The type hints use
Anybut it's not imported from thetypingmodule. This will fail when the module is loaded.🔧 Proposed fix
import asyncio import sys +from typing import Any from cloud_tasks.worker import Worker, WorkerDatametadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py (1)
73-75: Private attribute access pattern persists.Access to
worker._data.argswas previously flagged. This is the same pattern as inGO_0xxx_geometry_cloud.py.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
metadata_tools/column/COLUMNS_BODY.pymetadata_tools/columns.pymetadata_tools/common.pymetadata_tools/geometry_support.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.pymetadata_tools/hosts/GO_0xxx/gcp_geometry_config.ymlmetadata_tools/hosts/GO_0xxx/gcp_geometry_startup.shmetadata_tools/hosts/GO_0xxx/gcp_index_startup.shmetadata_tools/hosts/GO_0xxx/geometry_config.pymetadata_tools/hosts/GO_0xxx/templates/GO_0xxx_body_summary.lblmetadata_tools/index_support.pymetadata_tools/label_support.pymetadata_tools/util.pyrequirements-cloud.txt
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-07T23:57:15.800Z
Learnt from: jnspitale
Repo: SETI/rms-metadata-tools PR: 97
File: metadata_tools/columns.py:29-29
Timestamp: 2026-01-07T23:57:15.800Z
Learning: When using FCPath objects from rms-filecache with Python's built-in open(), convert the FCPath to a string using .as_posix() before passing it to open. FCPath does not implement the __fspath__ protocol, so use path_str = fcpath.as_posix() and then open(path_str) (or open(path_str, mode)) to ensure compatibility.
Applied to files:
metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.pymetadata_tools/columns.pymetadata_tools/label_support.pymetadata_tools/hosts/GO_0xxx/geometry_config.pymetadata_tools/common.pymetadata_tools/util.pymetadata_tools/index_support.pymetadata_tools/column/COLUMNS_BODY.pymetadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.pymetadata_tools/geometry_support.py
📚 Learning: 2026-01-16T16:21:24.025Z
Learnt from: jnspitale
Repo: SETI/rms-metadata-tools PR: 97
File: metadata_tools/templates/inventory.lbl:16-16
Timestamp: 2026-01-16T16:21:24.025Z
Learning: In metadata_tools/label_support.py, the PdsTemplate includes parameter always contains both defs.GLOBAL_TEMPLATE_PATH and template_dir (host-specific templates directory), regardless of the use_global_template flag. The use_global_template flag only determines which template file is used as the main template, not which directories are searched for $INCLUDE directives. Therefore, $INCLUDE statements can reference files in either the global templates directory or the host-specific templates directory without needing full paths.
Applied to files:
metadata_tools/label_support.py
📚 Learning: 2026-01-15T16:00:37.410Z
Learnt from: jnspitale
Repo: SETI/rms-metadata-tools PR: 97
File: metadata_tools/util.py:107-118
Timestamp: 2026-01-15T16:00:37.410Z
Learning: In metadata_tools/util.py, inline comments like "### move to utilities, why is this not in numpy?" refer to future work to move functions to a more global general utilities package outside of the metadata_tools project, not to the current metadata_tools.util module.
Applied to files:
metadata_tools/common.pymetadata_tools/util.pymetadata_tools/index_support.pymetadata_tools/geometry_support.py
🧬 Code graph analysis (4)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py (3)
metadata_tools/index_support.py (2)
process_index(568-616)get_args(448-473)metadata_tools/util.py (1)
parse_template_name(85-105)metadata_tools/common.py (1)
task_source(52-59)
metadata_tools/label_support.py (1)
metadata_tools/util.py (1)
get_template_name(72-82)
metadata_tools/util.py (2)
metadata_tools/common.py (1)
write(195-221)metadata_tools/geometry_support.py (1)
write(1420-1431)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py (3)
metadata_tools/geometry_support.py (2)
process_tables(1545-1650)get_args(1501-1542)metadata_tools/util.py (1)
parse_template_name(85-105)metadata_tools/common.py (1)
task_source(52-59)
🪛 Flake8 (7.3.0)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py
[error] 41-41: block comment should start with '# '
(E265)
[error] 43-43: undefined name 'Any'
(F821)
[error] 44-44: undefined name 'Any'
(F821)
[error] 54-54: block comment should start with '# '
(E265)
metadata_tools/columns.py
[error] 28-28: expected an indented block (comment)
(E115)
metadata_tools/hosts/GO_0xxx/geometry_config.py
[error] 21-21: block comment should start with '# '
(E265)
[error] 55-55: continuation line missing indentation or outdented
(E122)
[error] 56-56: continuation line missing indentation or outdented
(E122)
[error] 56-56: block comment should start with '# '
(E265)
[error] 68-68: continuation line over-indented for visual indent
(E127)
[error] 75-75: continuation line over-indented for visual indent
(E127)
[error] 79-79: continuation line over-indented for visual indent
(E127)
[error] 89-89: continuation line over-indented for visual indent
(E127)
[error] 95-95: block comment should start with '# '
(E265)
metadata_tools/common.py
[error] 20-20: continuation line under-indented for visual indent
(E128)
[error] 23-23: block comment should start with '# '
(E265)
[error] 39-39: block comment should start with '# '
(E265)
[error] 51-51: block comment should start with '# '
(E265)
[error] 61-61: block comment should start with '# '
(E265)
[error] 77-77: block comment should start with '# '
(E265)
[error] 95-95: block comment should start with '# '
(E265)
[error] 154-154: block comment should start with '# '
(E265)
[error] 194-194: block comment should start with '# '
(E265)
metadata_tools/util.py
[error] 523-523: SyntaxError: invalid syntax
(E999)
metadata_tools/index_support.py
[error] 25-25: block comment should start with '# '
(E265)
[error] 27-27: continuation line over-indented for visual indent
(E127)
[error] 567-567: block comment should start with '# '
(E265)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py
[error] 25-25: block comment should start with '# '
(E265)
[error] 27-27: undefined name 'Any'
(F821)
[error] 28-28: undefined name 'Any'
(F821)
[error] 41-41: block comment should start with '# '
(E265)
metadata_tools/geometry_support.py
[error] 97-97: continuation line missing indentation or outdented
(E122)
[error] 123-123: continuation line unaligned for hanging indent
(E131)
[error] 214-214: continuation line over-indented for visual indent
(E127)
[error] 217-217: continuation line over-indented for visual indent
(E127)
[error] 219-219: block comment should start with '# '
(E265)
[error] 257-257: block comment should start with '# '
(E265)
[error] 284-284: continuation line over-indented for visual indent
(E127)
[error] 314-314: block comment should start with '# '
(E265)
[error] 329-329: block comment should start with '# '
(E265)
[error] 361-361: block comment should start with '# '
(E265)
[error] 433-433: block comment should start with '# '
(E265)
[error] 452-452: block comment should start with '# '
(E265)
[error] 454-454: continuation line over-indented for visual indent
(E127)
[error] 455-455: continuation line over-indented for visual indent
(E127)
[error] 512-512: continuation line under-indented for visual indent
(E128)
[error] 513-513: continuation line under-indented for visual indent
(E128)
[error] 514-514: continuation line under-indented for visual indent
(E128)
[error] 515-515: continuation line under-indented for visual indent
(E128)
[error] 516-516: continuation line under-indented for visual indent
(E128)
[error] 517-517: continuation line under-indented for visual indent
(E128)
[error] 528-528: block comment should start with '# '
(E265)
[error] 608-608: continuation line over-indented for visual indent
(E127)
[error] 609-609: continuation line over-indented for visual indent
(E127)
[error] 610-610: continuation line over-indented for visual indent
(E127)
[error] 908-908: block comment should start with '# '
(E265)
[error] 1003-1003: block comment should start with '# '
(E265)
[error] 1030-1030: block comment should start with '# '
(E265)
[error] 1066-1066: block comment should start with '# '
(E265)
[error] 1101-1101: block comment should start with '# '
(E265)
[error] 1131-1131: block comment should start with '# '
(E265)
[error] 1161-1161: block comment should start with '# '
(E265)
[error] 1201-1201: block comment should start with '# '
(E265)
[error] 1235-1235: continuation line over-indented for visual indent
(E127)
[error] 1306-1306: block comment should start with '# '
(E265)
[error] 1338-1338: closing bracket does not match visual indentation
(E124)
[error] 1343-1343: block comment should start with '# '
(E265)
[error] 1378-1378: continuation line missing indentation or outdented
(E122)
🪛 Ruff (0.14.11)
metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py
43-43: Undefined name Any
(F821)
44-44: Undefined name Any
(F821)
metadata_tools/columns.py
30-30: Use of exec detected
(S102)
metadata_tools/hosts/GO_0xxx/geometry_config.py
96-96: Unused function argument: observation
(ARG001)
metadata_tools/common.py
32-32: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
70-70: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
70-70: Logging statement uses %
(G002)
96-96: Boolean default positional argument in function definition
(FBT002)
109-109: Missing return type annotation for special method __call__
(ANN204)
109-109: Unused method argument: parser
(ARG002)
109-109: Unused method argument: option_string
(ARG002)
150-150: Unnecessary parentheses after class definition
Remove parentheses
(UP039)
155-155: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
157-157: Boolean default positional argument in function definition
(FBT002)
189-189: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
195-195: Boolean default positional argument in function definition
(FBT002)
metadata_tools/util.py
523-523: Expected an identifier, but found a keyword from that cannot be used here
(invalid-syntax)
523-523: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
523-523: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
523-523: Multiple statements on one line (semicolon)
(E702)
523-523: Multiple statements on one line (semicolon)
(E702)
metadata_tools/index_support.py
26-26: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
27-27: Missing type annotation for **kwargs
(ANN003)
70-70: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
70-70: Logging statement uses %
(G002)
104-104: Boolean default positional argument in function definition
(FBT002)
395-395: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
395-395: Logging statement uses %
(G002)
399-399: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
399-399: Logging statement uses %
(G002)
405-405: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
405-405: Logging statement uses %
(G002)
476-476: Missing return type annotation for private function _create_index
Add return type annotation: None
(ANN202)
478-478: Boolean default positional argument in function definition
(FBT002)
483-483: Boolean default positional argument in function definition
(FBT002)
573-573: Boolean default positional argument in function definition
(FBT002)
metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py
27-27: Undefined name Any
(F821)
28-28: Undefined name Any
(F821)
metadata_tools/geometry_support.py
139-139: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
220-220: Missing return type annotation for private function _inventory
(ANN202)
234-234: Consider moving this statement to an else block
(TRY300)
258-258: Missing return type annotation for private function _select_bodies
(ANN202)
316-316: Missing return type annotation for staticmethod get_backplane_key
(ANN205)
347-347: Prefer next(iter(column_descs.keys())) over single element slice
Replace with next(iter(column_descs.keys()))
(RUF015)
373-373: Missing return type annotation for private function link_null
(ANN202)
434-434: Missing return type annotation for private function _get_system
(ANN202)
511-511: Unpacked variable overrides is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
529-529: Missing return type annotation for private function _prep_row
(ANN202)
697-697: Boolean positional value in function call
(FBT003)
702-702: Boolean positional value in function call
(FBT003)
729-729: Boolean positional value in function call
(FBT003)
879-879: Missing return type annotation for private function _circle_coverage
(ANN202)
909-909: Missing return type annotation for private function _formatted_column
(ANN202)
1004-1004: Missing return type annotation for private function _obs_excluded
(ANN202)
1031-1031: Missing return type annotation for private function _get_primary
(ANN202)
1067-1067: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1067-1067: Missing type annotation for **kwargs
(ANN003)
1102-1102: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1102-1102: Missing type annotation for **kwargs
(ANN003)
1132-1132: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1132-1132: Missing type annotation for **kwargs
(ANN003)
1162-1162: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1162-1162: Missing type annotation for **kwargs
(ANN003)
1202-1202: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1202-1202: Missing type annotation for **kwargs
(ANN003)
1234-1234: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1308-1308: Missing return type annotation for staticmethod get_override
(ANN205)
1345-1345: Missing return type annotation for staticmethod get_overrides
(ANN205)
1434-1434: Boolean default positional argument in function definition
(FBT002)
1474-1474: Use format specifiers instead of percent format
(UP031)
1474-1474: Logging statement uses %
(G002)
1554-1554: Boolean default positional argument in function definition
(FBT002)
🪛 Shellcheck (0.11.0)
metadata_tools/hosts/GO_0xxx/gcp_index_startup.sh
[info] 9-9: Not following: venv/bin/activate was not specified as input (see shellcheck -x).
(SC1091)
metadata_tools/hosts/GO_0xxx/gcp_geometry_startup.sh
[info] 8-8: Not following: venv/bin/activate was not specified as input (see shellcheck -x).
(SC1091)
🪛 YAMLlint (1.37.1)
metadata_tools/hosts/GO_0xxx/gcp_geometry_config.yml
[error] 3-3: too many spaces after colon
(colons)
[error] 4-4: too many spaces after colon
(colons)
[error] 5-5: too many spaces after colon
(colons)
[error] 6-6: too many spaces after colon
(colons)
[error] 7-7: too many spaces after colon
(colons)
[error] 8-8: too many spaces after colon
(colons)
[error] 9-9: too many spaces after colon
(colons)
🔇 Additional comments (20)
requirements-cloud.txt (1)
1-12: LGTM! Dependencies are appropriately pinned.The requirements file correctly specifies minimum versions for all dependencies. The
rms-pdstemplate>=2.3.0version constraint is valid.metadata_tools/common.py (2)
59-59: LGTM on task management functions.The
yield frompattern is correct and theadd_taskfunction properly logs and constructs task data. The typo in the docstring has been fixed.Also applies to: 62-75
176-191: LGTM on Table constructor initialization.The
template_pathandrowsare now properly initialized before the early return, preventingAttributeErroron subsequent method calls.metadata_tools/hosts/GO_0xxx/geometry_config.py (3)
16-21: LGTM on configuration constants.The configuration values for spacecraft ID, glob patterns, and exclusions are well-defined and clearly documented.
46-89: LGTM on MISSION_TABLE structure.The mission table correctly defines phase-specific configurations with SCLK ranges, exceptions, primary/secondary bodies, selections, and additions. The documented rules (lines 36-43) explain the selection logic clearly.
96-105: LGTM - docstring return type corrected.The return type in the docstring now correctly states
boolinstead ofstr.metadata_tools/util.py (4)
15-28: LGTM on PdsTable function.The unused variable is now properly prefixed with
_to indicate intentional discard. The comment explains the side-effect purpose (ensuring the .tab file is retrieved locally).
31-44: LGTM on select_dir logic.The function now correctly handles the collection/volume path construction with clear conditional logic.
384-422: LGTM on append_txt_file.The function correctly handles content normalization and writes the full string at once (not character-by-character as previously flagged).
539-556: LGTM on convert_mission_table.The function now accepts the
scparameter for spacecraft-specific SCLK conversion, addressing the previous hardcoded ID concern.metadata_tools/label_support.py (2)
13-17: LGTM on signature improvements.The function signature now properly uses keyword-only arguments for
use_global_templateandtable_type, addressing the earlier FBT002 lint concern. The indentation is consistent.
63-70: LGTM on template resolution and includes.The template path resolution logic correctly uses
host_template_dirfor non-global templates, and theincludesparameter now properly includes bothdefs.GLOBAL_TEMPLATE_PATHandhost_template_dirto support$INCLUDEdirectives from either location. Based on learnings, this aligns with the expected behavior where the includes parameter determines searchable directories regardless of theuse_global_templateflag.metadata_tools/column/COLUMNS_BODY.py (1)
5-6: No action required. Theutilmodule is actively used in the file at lines 88, 90, and 119 viautil.replacement_dict()andutil.replace()calls, confirming the import is necessary.metadata_tools/hosts/GO_0xxx/gcp_index_startup.sh (1)
1-12: LGTM! Previous review feedback has been addressed.The script now includes:
- A proper shebang (
#!/bin/bash)- Error handling on all
cdcommands (|| exit 1)- Actual GCS bucket paths instead of placeholders
The shellcheck hint about
venv/bin/activate(SC1091) is a false positive since shellcheck cannot follow dynamically sourced files.metadata_tools/index_support.py (3)
26-52: Constructor signature extension looks good.The addition of
template_pathandmetadata_dirparameters enables proper separation of concerns between input volumes, output directories, and metadata directories. The logger initialization and metadata_dir usage are correctly implemented.
104-136: LGTM! Pattern filtering logic is correct.The new
patternparameter for sub-selecting files is properly implemented usingfnmatch.filter. The earlycontinueon line 130 correctly skips non-matching files.
402-405: Previous security and deprecation concerns addressed.The code now uses
ast.literal_eval()instead ofeval()and catches the appropriate exceptions(ValueError, SyntaxError). The deprecatedlogger.warn()has been replaced withlogger.warning().metadata_tools/hosts/GO_0xxx/templates/GO_0xxx_body_summary.lbl (3)
5-5: Header note update looks good.The clarified note matches the template’s purpose.
33-44: Expanded DESCRIPTION reads well and adds useful context.The new narrative and conditional sentence improve clarity for cumulative vs. volume-specific usage.
26-26: No action needed. Include resolution for bare filenames is working correctly. Bothhost_defs.lbl(in the host-specific templates directory) andbody_summary_columns.lbl(in the global templates directory) are properly resolved because thePdsTemplateincludes parameter contains bothGLOBAL_TEMPLATE_PATHandhost_template_dirin all invocation contexts.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| # Write the task file | ||
| if task_list_only: | ||
| com.write_task_file(task_file) |
There was a problem hiding this comment.
Task file written inside the loop - potential bug.
The com.write_task_file(task_file) call on lines 559-560 is inside the for root, dirs, _files in volume_tree.walk() loop. This means the task file will be written on every directory iteration, potentially before all volumes have been added to the task list.
The call should be moved outside the loop to ensure all volumes are collected before writing.
🔧 Proposed fix
# Log a warning for any columns that never had non-null values
if unused:
logger.warning('Unused columns: %s', unused)
- logger.close(force=True)
+
+ # Write the task file after processing all volumes
+ if task_list_only:
+ com.write_task_file(task_file)
+
+ logger.close(force=True)Move the task file writing and logger close outside the for loop.
🤖 Prompt for AI Agents
In `@metadata_tools/index_support.py` around lines 558 - 560, The call to
com.write_task_file(task_file) (guarded by task_list_only) is currently inside
the volume_tree.walk() loop and causes the task file to be written repeatedly
before all volumes are collected; move the entire write operation (and the
subsequent logger.close() if present) outside and after the for root, dirs,
_files in volume_tree.walk(): loop so that write_task_file(task_file) is
executed once after volume collection is complete, keeping the task_list_only
conditional unchanged.
| # if not task_list_only: | ||
| # print('testing', args.volumes) | ||
| # return |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove commented debug code before merge.
These commented lines appear to be debug/test code that should be removed.
🧹 Proposed fix
if args is None:
parser = get_args(host=host, index_type=index_type)
args = parser.parse_args()
-# if not task_list_only:
-# print('testing', args.volumes)
-# return
-
if volumes is None:🤖 Prompt for AI Agents
In `@metadata_tools/index_support.py` around lines 600 - 602, Remove the leftover
commented debug block that checks "task_list_only" and prints "testing" with
"args.volumes" — specifically delete the commented lines "# if not
task_list_only:", "# print('testing', args.volumes)", and "#
return" in metadata_tools/index_support.py so no debug scaffolding remains;
ensure no dependent logic relies on that commented branch and run tests to
confirm behavior unchanged.
| dir = filepath.parent | ||
| body = filepath.stem | ||
| label_path = dir / (body + '.lbl') | ||
| host_template_dir = host_template_path.parent |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider validating host_template_path before accessing .parent.
If host_template_path is None or an unexpected type, calling .parent will raise an AttributeError. Since this is a required parameter and other paths like filepath are wrapped in FCPath(), consider doing the same here for consistency and safety.
♻️ Proposed fix
+ host_template_path = FCPath(host_template_path)
host_template_dir = host_template_path.parent📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| host_template_dir = host_template_path.parent | |
| host_template_path = FCPath(host_template_path) | |
| host_template_dir = host_template_path.parent |
🤖 Prompt for AI Agents
In `@metadata_tools/label_support.py` at line 43, Validate and normalize
host_template_path before accessing .parent: ensure host_template_path is not
None and is converted to an FCPath-like object (consistent with how filepath is
handled) so you don't call .parent on an unexpected type; update the code that
computes host_template_dir (referencing host_template_path and
host_template_dir) to wrap/validate host_template_path (e.g., via FCPath or a
type check and clear error) and then read .parent only after successful
normalization.
metadata_tools/util.py
Outdated
| def sclk_to_ticks(sclk, sc): | ||
| """Convert spacecraft clock count string to ticks. | ||
| Args: | ||
| sclk (list): Spacecraft clock count string. | ||
| bases (list): Base (int) to use for each decimal place. | ||
| sc (int): NAIF spacecraft identifier. | ||
| Returns: | ||
| int: Spacecraft clock ticks. | ||
| """ | ||
| return cspyce. from IPython import embed; print('+++++++++++++'); embed() | ||
| (sc, sclk) |
There was a problem hiding this comment.
Critical: Debug code left in production - causes SyntaxError.
Lines 523-524 contain leftover debug code (from IPython import embed; print('+++++++++++++'); embed()) embedded in an invalid position within the cspyce. call. This will cause a SyntaxError when the module is imported, breaking the entire package.
🐛 Proposed fix
def sclk_to_ticks(sclk, sc):
"""Convert spacecraft clock count string to ticks.
Args:
sclk (list): Spacecraft clock count string.
sc (int): NAIF spacecraft identifier.
Returns:
int: Spacecraft clock ticks.
"""
- return cspyce. from IPython import embed; print('+++++++++++++'); embed()
-(sc, sclk)
+ return cspyce.sctiks_alias(sc, sclk)🧰 Tools
🪛 Flake8 (7.3.0)
[error] 523-523: SyntaxError: invalid syntax
(E999)
🪛 Ruff (0.14.11)
523-523: Expected an identifier, but found a keyword from that cannot be used here
(invalid-syntax)
523-523: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
523-523: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
523-523: Multiple statements on one line (semicolon)
(E702)
523-523: Multiple statements on one line (semicolon)
(E702)
🤖 Prompt for AI Agents
In `@metadata_tools/util.py` around lines 513 - 524, Remove the leftover debug
injection inside sclk_to_ticks and replace it with a proper cspyce conversion
call: delete the "from IPython import embed; print('+++++++++++++'); embed()"
and the malformed "cspyce." fragment, then call the appropriate cspyce function
(e.g., cspyce.sclk2et or cspyce.sclk2ticks depending on your API) using the sc
and sclk parameters and return the integer tick/ET value so sclk_to_ticks
returns a valid int; ensure you reference the sclk_to_ticks function and cspyce
namespace when making this fix.
| def get_observation_id(observation): | ||
| """Utility function to determine the observation ID for an observation. | ||
| # Compute ticks | ||
| ticks = fields[-1] | ||
| for i in range(len(fields)-1): | ||
| ticks += fields[i]*bases[i+1] | ||
| Args: | ||
| observation_id (str): Observation ID. | ||
| return ticks | ||
| Returns: | ||
| None. | ||
| """ | ||
| return str(observation.subfields['dict']['OBSERVATION_ID']) |
There was a problem hiding this comment.
Docstring parameter name mismatch.
The docstring says observation_id (str) but the parameter is observation. Also, the return type says None but the function returns a string.
📝 Proposed fix
def get_observation_id(observation):
"""Utility function to determine the observation ID for an observation.
Args:
- observation_id (str): Observation ID.
+ observation (oops.Observation): OOPS Observation object.
Returns:
- None.
+ str: Observation ID from the observation's dictionary.
"""
return str(observation.subfields['dict']['OBSERVATION_ID'])🤖 Prompt for AI Agents
In `@metadata_tools/util.py` around lines 527 - 536, The docstring for
get_observation_id is incorrect: it mentions a parameter observation_id and a
return type of None even though the function accepts observation and returns a
string; update the docstring to document the actual parameter name "observation"
(describe its expected type/structure) and change the Returns section to reflect
that a string (the observation ID) is returned so the docstring matches the
function behavior.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
metadata_tools/util.py (2)
72-82: Update docstring to document all parameters.The docstring is incomplete - it only documents
filenamebut the function now takesfilename,volume_id, andcode_dirparameters.📝 Proposed fix
def get_template_name(filename, volume_id, code_dir): """Determine the name of the label template. Args: filename (str): Name of table or label file. + volume_id (str): Volume identifier to replace. + code_dir (FCPath): Code directory whose name provides the collection. Returns: str: Index name. """
297-309: Bug:as_posix()called on string will raiseAttributeError.At line 306,
resultis a string (afteros.path.expandvars()), but the code callsresult.as_posix()which will fail since strings don't have this method. The function should returnresultdirectly for string inputs.🐛 Proposed fix
if isinstance(filespec, str): - return result.as_posix() + return result if isinstance(filespec, FCPath): return FCPath(result) return Path(result)
🤖 Fix all issues with AI agents
In `@metadata_tools/column/COLUMNS_BODY.py`:
- Around line 56-57: The tuple for "limb_clock_angle" was mistakenly made
nested; change its second element to the same simple parameter used by
"limb_altitude" (replace ("limb_altitude", defs.BODYX, -0.01, 3, True) with
defs.BODYX) so the entry becomes (("limb_clock_angle", defs.BODYX, -0.01, 3,
True), ("", "", "")), or alternately update the evaluation code in
geometry_support.py to detect and unpack a nested tuple for event_key[1] before
comparing to defs.NULL and before using it as a dict key (ensure
excluded_mask_dict[(target,) + mask_desc] uses the unpacked/simple key).
In `@metadata_tools/common.py`:
- Around line 155-221: The write() method assumes self.filename exists but
__init__ can return early without setting it; update write() (in the same class
as __init__) to guard usage of self.filename (use hasattr(self, "filename") or
check if self.filename is None) and early-return (no-op) if it's not set before
calling util.write_txt_file or lab.create; ensure you skip both the table write
(util.write_txt_file) and the label creation (lab.create) when filename is
missing and adjust the logger.info call accordingly.
In `@metadata_tools/geometry_support.py`:
- Around line 932-953: In _formatted_column, the branch that handles string
values currently leaves values as a bare str causing per-character iteration;
when isinstance(values, str) replace results = values with results = [values] *
number_of_values (or equivalently [values for _ in range(number_of_values)]) so
the string is treated as a single cell repeated to match number_of_values; use
the existing variables values, number_of_values and results to implement this
change.
- Around line 308-313: The current sort of body_names uses key=lambda name:
list(col.BODIES.keys()).index(name) which will raise if a name is not present in
col.BODIES; change the sort key to handle missing entries safely—for example,
compute a mapping of body order = {name: idx for idx, name in
enumerate(col.BODIES.keys())} and then sort with key=lambda name:
order.get(name, len(order) + hash(name) % 1000) or use a tuple key like
key=lambda name: (0, order[name]) if name in order else (1, name) so unknown
bodies get a stable fallback ordering; update the code around
body_names.sort(...) to use that safe key (referencing body_names, col.BODIES,
and the sort key lambda).
- Around line 375-414: The current grouping uses link as dict key which
overwrites groups with the same link but different link_id; change the grouping
to key by (link, link_id) and store a list of associated backplane keys (e.g.
value {'backplane_keys': [...], 'null_value': null_value, 'link_name': link})
instead of a single backplane_key so all columns in a link group are tracked;
update link_null (and any other link_* functions) to accept the link spec with
the list of backplane keys (use the stored 'backplane_keys' list) and iterate
those indices to set nulls; when invoking the link function, extract the
link_name to build the function name (locals()['link_' + link_name]) and pass
the group spec, the global backplane_keys and data_columns as before.
- Around line 1237-1261: In the __init__ constructor of the geometry Suite class
the code unconditionally calls FCPath(metadata_dir) even though metadata_dir
defaults to None; change this to defensively handle None by only wrapping
metadata_dir with FCPath when it's not None (e.g., if metadata_dir is None set
self.metadata_dir = None else self.metadata_dir = FCPath(metadata_dir)), or
alternatively remove the None default and require metadata_dir as a non-optional
parameter—update the assignment to self.metadata_dir and adjust the
signature/docstring accordingly; refer to the __init__ method and the FCPath and
metadata_dir symbols to locate the fix.
In `@metadata_tools/hosts/GO_0xxx/gcp_index_startup.sh`:
- Around line 1-7: The git clone URL in the startup script is malformed: replace
the incorrect repository string
"https://github.com/SETI/rms-metadata-tools.gitnew_overrides" used in the git
clone command with the correct URL
"https://github.com/SETI/rms-metadata-tools.git" (and if you intended to use the
new_overrides branch, add an explicit checkout of that branch after cloning,
e.g. run git checkout new_overrides in the rms-metadata-tools repo).
♻️ Duplicate comments (6)
metadata_tools/util.py (1)
526-535: Docstring parameter and return type mismatch.The docstring states the parameter is
observation_id (str)but the actual parameter isobservation. The return type saysNonebut the function returns a string.📝 Proposed fix
def get_observation_id(observation): """Utility function to determine the observation ID for an observation. Args: - observation_id (str): Observation ID. + observation (oops.Observation): OOPS Observation object. Returns: - None. + str: Observation ID from the observation's dictionary. """ return str(observation.subfields['dict']['OBSERVATION_ID'])metadata_tools/hosts/GO_0xxx/gcp_geometry_startup.sh (1)
10-12: Fix Python script path - file is not at repository root.The script runs
python3 GO_0xxx_geometry_cloud.pybut aftercd rms-metadata-tools, the Python script is located atmetadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py, not at the repository root.🐛 Proposed fix
pip install -r requirements.txt -python3 GO_0xxx_geometry_cloud.py gs://rms-metadata-jspitale/metadata_test/GO_0xxx/ \ +python3 metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py gs://rms-metadata-jspitale/metadata_test/GO_0xxx/ \ gs://rms-node-holdings/pds3-holdings/metadata/GO_0xxx/ \ gs://rms-metadata-jspitale/metadata_test/GO_0xxx/metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py (1)
66-68: Accessing private_dataattribute - document the workaround.The code accesses
worker._data.argswhich uses a private attribute. Per previous investigation, this is the documented way to access parsed arguments from therms-cloud-tasksWorker class. Consider adding a brief comment noting this is the intended interface until the library provides a public accessor.📝 Suggested comment
# set up the task file containing one entry per volume + # Note: _data.args is the documented way to access parsed arguments from Worker process_tables(hconf.template_name, glob=config.glob, index_glob=config.index_glob, selection=config.selection, exclude=config.exclude, args=worker._data.args, task_list_only=True, task_file=worker._data.args.task_file)metadata_tools/hosts/GO_0xxx/GO_0xxx_index_cloud.py (2)
42-55: Fix block comment separators to satisfy linting.PEP 8 expects a space after
#; these separators are still flagged by lint.📝 Proposed fix
-#======================================================================================== +# ======================================================================================= ... -#======================================================================================== +# =======================================================================================
71-76: Avoid reaching intoworker._data.args; use a public accessor or parsed args.Direct access to a private attribute couples this module to
cloud_tasksinternals and can break on library updates. Prefer a public accessor (if available) or parse args explicitly and reuse that object when callingprocess_index.metadata_tools/common.py (1)
138-140: Make--labelsa real flag.
type=boolforces a value and treats any non-empty string asTrue, which contradicts “If given…”. Preferaction='store_true'(or allow explicit values with a custom parser).🔧 Proposed fix (flag semantics)
- gr.add_argument('--labels', '-l', type=bool, metavar='labels', - default=False, - help='''If given, labels are generated for existing files.''') + gr.add_argument('--labels', '-l', action='store_true', + help='''If given, labels are generated for existing files.''')
| (("limb_altitude", defs.BODYX, -0.01, 3, True), ("", "", "")), | ||
| (("limb_clock_angle", ("limb_altitude", defs.BODYX, -0.01, 3, True)), ("", "", "")), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for limb_clock_angle to understand its usage context
rg -n "limb_clock_angle" --type=py -C 3Repository: SETI/rms-metadata-tools
Length of output: 1672
🏁 Script executed:
#!/bin/bash
# Search for Backplane class and evaluate method
rg -n "class Backplane" --type=py
rg -n "def evaluate" --type=py | head -20Repository: SETI/rms-metadata-tools
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for COLUMNS_BODY usage
rg -n "COLUMNS_BODY" --type=py -C 2
# Also search for how columns are evaluated or processed
rg -n "evaluate\|process.*column" --type=py | grep -v ".pyc" | head -30Repository: SETI/rms-metadata-tools
Length of output: 1280
🏁 Script executed:
#!/bin/bash
# Read COLUMNS_BODY.py to understand the structure
cat -n metadata_tools/column/COLUMNS_BODY.py | head -70
# Search for backplane modules
fd "backplane" --type=fRepository: SETI/rms-metadata-tools
Length of output: 4798
🏁 Script executed:
#!/bin/bash
# Search for Backplane class in Python files
rg -n "class Backplane" --type=py
# Search for imports of Backplane
rg -n "from.*Backplane\|import.*Backplane" --type=py
# Look for usage/calls to Backplane.evaluate
rg -n "\.evaluate\(" --type=py -C 2 | head -40Repository: SETI/rms-metadata-tools
Length of output: 2570
🏁 Script executed:
#!/bin/bash
# Search for backplane instantiation
rg -n "backplane\s*=" --type=py -C 2 | head -50
# Search for Backplane in imports or usage
rg -n "backplane\s*=" --type=pyRepository: SETI/rms-metadata-tools
Length of output: 577
🏁 Script executed:
#!/bin/bash
# Search for any special handling of nested tuples
rg -n "limb_clock_angle\|nested.*tuple\|tuple.*nested" --type=py
# Look for geometry or backplane module documentation
find . -name "*.md" -o -name "*.txt" | xargs grep -l "backplane\|Backplane" 2>/dev/null
# Search for how different column definitions are processed
rg -n "event_key\|backplane_key" --type=py -C 2Repository: SETI/rms-metadata-tools
Length of output: 122
🏁 Script executed:
#!/bin/bash
# Look at the code around backplane.evaluate calls
sed -n '695,710p' metadata_tools/geometry_support.py
# Also search for how columns are extracted and used
rg -n "backplane_key\|BODY_COLUMNS" --type=py -B 2 -A 2Repository: SETI/rms-metadata-tools
Length of output: 692
🏁 Script executed:
#!/bin/bash
# Look at how column_desc is processed to extract event_key
sed -n '680,720p' metadata_tools/geometry_support.pyRepository: SETI/rms-metadata-tools
Length of output: 1722
🏁 Script executed:
#!/bin/bash
# Search for test files
find . -name "*test*.py" -o -name "test_*" | head -20
# Look for limb_clock_angle in tests
rg -n "limb_clock_angle" --type=py
# Search for any TODOs or FIXMEs related to this
rg -n "TODO\|FIXME\|BUG\|HACK" --type=py | grep -i "limb\|clock\|nested"Repository: SETI/rms-metadata-tools
Length of output: 648
🏁 Script executed:
#!/bin/bash
# Check test_geometry.py for limb_clock_angle tests
cat -n tests/test_geometry.py | head -100Repository: SETI/rms-metadata-tools
Length of output: 4994
🏁 Script executed:
#!/bin/bash
# Continue reading test_geometry.py to find limb_clock_angle related tests
cat -n tests/test_geometry.py | grep -i "limb\|clock"
# Also check the entire file length
wc -l tests/test_geometry.pyRepository: SETI/rms-metadata-tools
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Read the full test_geometry.py file
cat -n tests/test_geometry.pyRepository: SETI/rms-metadata-tools
Length of output: 8443
🏁 Script executed:
#!/bin/bash
# Check what defs.NULL is
rg -n "NULL\s*=" metadata_tools/defs.py
# Look for excluded_mask_dict to understand its structure
rg -n "excluded_mask_dict" --type=py -B 2 -A 2Repository: SETI/rms-metadata-tools
Length of output: 1687
🏁 Script executed:
#!/bin/bash
# Search for backplane module in oops or any backplane implementation in this repo
find . -name "*backplane*" -type f
# Check if oops is imported and how
rg -n "import oops\|from oops" --type=pyRepository: SETI/rms-metadata-tools
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check git history for the limb_clock_angle change
git log --oneline -n 50 -- metadata_tools/column/COLUMNS_BODY.py 2>/dev/null | head -20
# Also check if there are any comments in the code around this change
sed -n '54,60p' metadata_tools/column/COLUMNS_BODY.py
# Look for any PR or commit messages mentioning limb_clock_angle
git log -p -- metadata_tools/column/COLUMNS_BODY.py 2>/dev/null | grep -A 5 -B 5 "limb_clock_angle" | head -50Repository: SETI/rms-metadata-tools
Length of output: 545
The nested tuple structure for limb_clock_angle breaks column evaluation logic.
The change at line 57 creates a critical issue: the second element of the tuple is now a nested tuple ("limb_altitude", defs.BODYX, -0.01, 3, True) instead of a simple object like defs.BODYX.
This breaks the code at geometry_support.py:706 which compares event_key[1] == defs.NULL (expecting a string), and critically fails at line 711 where event_key[1] is used as a dictionary key in excluded_mask_dict[(target,) + mask_desc]. Using a nested tuple as a dictionary key will cause the lookup to fail or use an incorrect key, preventing proper mask evaluation for the limb_clock_angle column.
Either revert this to a simple parameter structure matching limb_altitude (line 56), or implement proper handling in the evaluation logic to unpack nested tuples before comparison and dictionary access.
🤖 Prompt for AI Agents
In `@metadata_tools/column/COLUMNS_BODY.py` around lines 56 - 57, The tuple for
"limb_clock_angle" was mistakenly made nested; change its second element to the
same simple parameter used by "limb_altitude" (replace ("limb_altitude",
defs.BODYX, -0.01, 3, True) with defs.BODYX) so the entry becomes
(("limb_clock_angle", defs.BODYX, -0.01, 3, True), ("", "", "")), or alternately
update the evaluation code in geometry_support.py to detect and unpack a nested
tuple for event_key[1] before comparing to defs.NULL and before using it as a
dict key (ensure excluded_mask_dict[(target,) + mask_desc] uses the
unpacked/simple key).
| def __init__(self, output_dir=None, template_path=None, | ||
| volume_id=None, level=None, qualifier=None, prefix=None, | ||
| suffix=None, use_global_template=False): | ||
| """Constructor for a table object. | ||
| Args: | ||
| output_dir (str, Path, or FCPath): | ||
| Directory in which to write the index files. | ||
| template_path (str, Path, or FCPath): Path to the host template. | ||
| volume_id (str): Volume ID. | ||
| level (str, optional): | ||
| Processing level: "summary", "detailed", or "index". | ||
| qualifier (str): | ||
| "sky", "sun", "ring", "body", "inventory", or "supplemental". | ||
| prefix (str): File path prefix. | ||
| suffix (str): File name suffix. | ||
| use_global_template (bool): | ||
| If True, the label template is to be found in the global | ||
| template directory. | ||
| """ | ||
| self.template_path = None | ||
| if template_path: | ||
| self.template_path = FCPath(template_path) | ||
| self.volume_id = volume_id | ||
| self.level = level | ||
| self.qualifier = qualifier | ||
| self.use_global_template = use_global_template | ||
| self.rows = [] | ||
|
|
||
| if not output_dir: | ||
| return | ||
|
|
||
| if not suffix: | ||
| suffix = "_%s_%s.tab" % (self.qualifier, self.level) | ||
| prefix = output_dir.joinpath(self.volume_id).as_posix() | ||
| self.filename = FCPath(prefix + suffix) | ||
|
|
||
|
|
||
| #===================================================================================== | ||
| def write(self, labels_only=False): | ||
| """Write a table and its label. | ||
| Args: | ||
| labels_only (bool, optional): | ||
| If True, labels are generated for any existing geometry | ||
| tables. | ||
| Returns: | ||
| None. | ||
| """ | ||
| logger = get_logger() | ||
|
|
||
| if not labels_only: | ||
| if not self.rows: | ||
| return | ||
|
|
||
| # Write table | ||
| logger.info("Writing:", self.filename) | ||
| util.write_txt_file(self.filename, self.rows) | ||
|
|
||
| # Write label | ||
| table_type = self.qualifier | ||
| if self.level: | ||
| table_type += '_' + self.level | ||
| lab.create(self.filename, self.template_path, | ||
| table_type=table_type, use_global_template=self.use_global_template) |
There was a problem hiding this comment.
Guard self.filename when output_dir is omitted.
__init__ returns early without defining self.filename, but write() always reads it. This will raise AttributeError if a Table is constructed without an output directory.
🔧 Proposed fix
self.use_global_template = use_global_template
self.rows = []
+ self.filename = None
if not output_dir:
return
...
def write(self, labels_only=False):
"""Write a table and its label.
@@
"""
logger = get_logger()
+ if self.filename is None:
+ return🧰 Tools
🪛 Flake8 (7.3.0)
[error] 194-194: block comment should start with '# '
(E265)
🪛 Ruff (0.14.11)
155-155: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
157-157: Boolean default positional argument in function definition
(FBT002)
189-189: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
195-195: Boolean default positional argument in function definition
(FBT002)
🤖 Prompt for AI Agents
In `@metadata_tools/common.py` around lines 155 - 221, The write() method assumes
self.filename exists but __init__ can return early without setting it; update
write() (in the same class as __init__) to guard usage of self.filename (use
hasattr(self, "filename") or check if self.filename is None) and early-return
(no-op) if it's not set before calling util.write_txt_file or lab.create; ensure
you skip both the table write (util.write_txt_file) and the label creation
(lab.create) when filename is missing and adjust the logger.info call
accordingly.
| # Cull duplicate bodies and verify all bodies are in the registry | ||
| body_names = list(dict.fromkeys(body_names)) | ||
|
|
||
| # Sort bodies based on occurence in BODIES list | ||
| body_names.sort(key=lambda name : list(col.BODIES.keys()).index(name)) | ||
|
|
There was a problem hiding this comment.
Prevent crash when a target isn’t in col.BODIES.
list(col.BODIES.keys()).index(name) will raise if name isn’t in the registry (e.g., targeted irregular bodies added later). Use a safe ordering fallback.
🔧 Proposed fix
# Cull duplicate bodies and verify all bodies are in the registry
body_names = list(dict.fromkeys(body_names))
# Sort bodies based on occurence in BODIES list
- body_names.sort(key=lambda name : list(col.BODIES.keys()).index(name))
+ body_order = {name: i for i, name in enumerate(col.BODIES.keys())}
+ body_names.sort(key=lambda name: body_order.get(name, len(body_order)))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Cull duplicate bodies and verify all bodies are in the registry | |
| body_names = list(dict.fromkeys(body_names)) | |
| # Sort bodies based on occurence in BODIES list | |
| body_names.sort(key=lambda name : list(col.BODIES.keys()).index(name)) | |
| # Cull duplicate bodies and verify all bodies are in the registry | |
| body_names = list(dict.fromkeys(body_names)) | |
| # Sort bodies based on occurence in BODIES list | |
| body_order = {name: i for i, name in enumerate(col.BODIES.keys())} | |
| body_names.sort(key=lambda name: body_order.get(name, len(body_order))) |
🤖 Prompt for AI Agents
In `@metadata_tools/geometry_support.py` around lines 308 - 313, The current sort
of body_names uses key=lambda name: list(col.BODIES.keys()).index(name) which
will raise if a name is not present in col.BODIES; change the sort key to handle
missing entries safely—for example, compute a mapping of body order = {name: idx
for idx, name in enumerate(col.BODIES.keys())} and then sort with key=lambda
name: order.get(name, len(order) + hash(name) % 1000) or use a tuple key like
key=lambda name: (0, order[name]) if name in order else (1, name) so unknown
bodies get a stable fallback ordering; update the code around
body_names.sort(...) to use that safe key (referencing body_names, col.BODIES,
and the sort key lambda).
| def link_null(link, backplane_keys, data_columns): | ||
| """Enter null value for all linked columns if any of them are null. | ||
| Args: | ||
| link (dict): Defines the link: | ||
| backplane_key : Linked backplane key. | ||
| null_value : Null value for this key. | ||
| backplane_keys (list): All backplane keys. | ||
| data_columns (list): Column values for each backplane key. | ||
| Returns: | ||
| Update data columns | ||
| """ | ||
| # Locate the linked columns | ||
| ii = [i for i, key in enumerate(backplane_keys) if key==link['backplane_key']] | ||
|
|
||
| for i in range(len(ii)): | ||
| val = data_columns[ii[i]] | ||
| if float(val) == link['null_value']: | ||
| for i in range(len(ii)): | ||
| data_columns[ii[i]] = val | ||
|
|
||
| return data_columns | ||
|
|
||
| # Get the backplane key mapping | ||
| backplane_keys, data_columns = self.get_key_map(columns, qualifier) | ||
|
|
||
| # Build link dictionary | ||
| links = {} | ||
| for key in backplane_keys: | ||
| format = FORMAT_DICT[key] | ||
| (_,_,_,_,_, null_value, _, _, link_id, link) = format | ||
| if link_id: | ||
| links[link] = {'backplane_key' : key, | ||
| 'null_value' : null_value} | ||
|
|
||
| # Call link functions | ||
| for link in links: | ||
| link_fn = locals()['link_' + link] | ||
| data_columns = link_fn(links[link], backplane_keys, data_columns) |
There was a problem hiding this comment.
Honor link_id when building linked column groups.
The format spec says columns with the same link and link_id should be linked, but the current dict keying by link overwrites groups and only tracks one key. Group by (link, link_id) and pass all keys into the link function.
🔧 Proposed fix
def link_null(link, backplane_keys, data_columns):
@@
- ii = [i for i, key in enumerate(backplane_keys) if key==link['backplane_key']]
+ ii = [i for i, key in enumerate(backplane_keys)
+ if key in link['backplane_keys']]
- for i in range(len(ii)):
- val = data_columns[ii[i]]
- if float(val) == link['null_value']:
- for i in range(len(ii)):
- data_columns[ii[i]] = val
+ for idx in ii:
+ val = data_columns[idx]
+ if float(val) == link['null_value']:
+ for j in ii:
+ data_columns[j] = val
+ break
@@
- links = {}
+ links = {}
for key in backplane_keys:
format = FORMAT_DICT[key]
(_,_,_,_,_, null_value, _, _, link_id, link) = format
if link_id:
- links[link] = {'backplane_key' : key,
- 'null_value' : null_value}
+ group = links.setdefault((link, link_id),
+ {'backplane_keys': [], 'null_value': null_value})
+ group['backplane_keys'].append(key)
@@
- for link in links:
- link_fn = locals()['link_' + link]
- data_columns = link_fn(links[link], backplane_keys, data_columns)
+ for (link, _link_id), link_def in links.items():
+ link_fn = locals()['link_' + link]
+ data_columns = link_fn(link_def, backplane_keys, data_columns)🧰 Tools
🪛 Ruff (0.14.11)
375-375: Missing return type annotation for private function link_null
(ANN202)
🤖 Prompt for AI Agents
In `@metadata_tools/geometry_support.py` around lines 375 - 414, The current
grouping uses link as dict key which overwrites groups with the same link but
different link_id; change the grouping to key by (link, link_id) and store a
list of associated backplane keys (e.g. value {'backplane_keys': [...],
'null_value': null_value, 'link_name': link}) instead of a single backplane_key
so all columns in a link group are tracked; update link_null (and any other
link_* functions) to accept the link spec with the list of backplane keys (use
the stored 'backplane_keys' list) and iterate those indices to set nulls; when
invoking the link function, extract the link_name to build the function name
(locals()['link_' + link_name]) and pass the group spec, the global
backplane_keys and data_columns as before.
| if not isinstance(values, str): | ||
| if number_of_values == 1: | ||
| meanval = values.mean().as_builtin() | ||
| if isinstance(meanval, oops.Scalar) and meanval.mask: | ||
| results = [null_value] | ||
| else: | ||
| results = [meanval] | ||
|
|
||
| elif np.all(values.mask): | ||
| results = [null_value, null_value] | ||
|
|
||
| elif flag == "360": | ||
| results = self._circle_coverage(values, null_value) | ||
|
|
||
| elif flag == "-180": | ||
| results = self._circle_coverage(values, null_value, flag=flag) | ||
|
|
||
| else: | ||
| results = [meanval] | ||
|
|
||
| elif np.all(values.mask): | ||
| results = [null_value, null_value] | ||
|
|
||
| elif flag == "360": | ||
| results = util._get_range_mod360(values) | ||
|
|
||
| elif flag == "-180": | ||
| results = util._get_range_mod360(values, alt_format=flag) | ||
|
|
||
| results = [values.min().as_builtin(), values.max().as_builtin()] | ||
| else: | ||
| results = [values.min().as_builtin(), values.max().as_builtin()] | ||
| results = values | ||
|
|
There was a problem hiding this comment.
String nulls expand into per-character columns.
When values is a string (e.g., null_value), _formatted_column iterates characters and emits too many columns. Wrap the string into a list sized by number_of_values.
🔧 Proposed fix
- else:
- results = values
+ else:
+ results = [values] * number_of_values🤖 Prompt for AI Agents
In `@metadata_tools/geometry_support.py` around lines 932 - 953, In
_formatted_column, the branch that handles string values currently leaves values
as a bare str causing per-character iteration; when isinstance(values, str)
replace results = values with results = [values] * number_of_values (or
equivalently [values for _ in range(number_of_values)]) so the string is treated
as a single cell repeated to match number_of_values; use the existing variables
values, number_of_values and results to implement this change.
| def __init__(self, input_dir, output_dir, template_path, metadata_dir=None, | ||
| selection='', glob=None, index_glob=None, first=None, sampling=8): | ||
| """Constructor for a geometry Suite object. | ||
| Args: | ||
| input_dir (str, Path, or FCPath): Directory containing the volume. | ||
| output_dir (str, Path, or FCPath): | ||
| Directory in which to write the geometry files. | ||
| template_path (str, Path, or FCPath): Path to the host template. | ||
| selection (str, optional): | ||
| A string containing... | ||
| "S" to generate summary files; | ||
| "D" to generate detailed files. | ||
| glob (str, optional): Glob pattern for index files. | ||
| glob (str, optional): Glob pattern for data files. | ||
| index_glob (str, optional): Glob pattern for index files. | ||
| first (bool, optional): | ||
| If given, at most this many files are processed in each volume. | ||
| sampling (int, optional): Pixel sampling density. | ||
| """ | ||
| logger = meta.get_logger() | ||
|
|
||
| # Save inputs | ||
| self.input_dir = FCPath(input_dir) | ||
| self.output_dir = FCPath(output_dir) | ||
| self.metadata_dir = FCPath(metadata_dir) | ||
| self.template_path = FCPath(template_path) | ||
| self.glob = glob |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the file and check its structure
fd -t f geometry_support.py
# Check the FCPath class definition
rg -n "class FCPath" --type=py -A 5
# Check the Suite.__init__ method
rg -n "class Suite" --type=py -A 30Repository: SETI/rms-metadata-tools
Length of output: 190
🏁 Script executed:
#!/bin/bash
# Look for all instantiations of Suite class
rg -n -C2 "Suite\(" --type=py | head -100Repository: SETI/rms-metadata-tools
Length of output: 983
🏁 Script executed:
#!/bin/bash
# Check the actual metadata_dir usage in the Suite class
rg -n "self.metadata_dir" metadata_tools/geometry_support.py -A 2 -B 2Repository: SETI/rms-metadata-tools
Length of output: 501
🏁 Script executed:
#!/bin/bash
# Check FCPath implementation to understand None handling
rg -n "class FCPath" --type=py -A 20Repository: SETI/rms-metadata-tools
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Find where FCPath is imported from
rg -n "import.*FCPath|from.*FCPath" metadata_tools/geometry_support.pyRepository: SETI/rms-metadata-tools
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Check the context around the Suite instantiation at line 1646
sed -n '1630,1660p' metadata_tools/geometry_support.pyRepository: SETI/rms-metadata-tools
Length of output: 1144
🏁 Script executed:
#!/bin/bash
# Check if FCPath is imported and what module it comes from
head -100 metadata_tools/geometry_support.py | rg -n "import|from"Repository: SETI/rms-metadata-tools
Length of output: 488
🏁 Script executed:
#!/bin/bash
# Check where metadata_dir is defined in the function containing line 1646
sed -n '1500,1650p' metadata_tools/geometry_support.py | rg -n "metadata_dir"Repository: SETI/rms-metadata-tools
Length of output: 225
🏁 Script executed:
#!/bin/bash
# Get more context to see the full function and metadata_dir initialization
sed -n '1550,1650p' metadata_tools/geometry_support.pyRepository: SETI/rms-metadata-tools
Length of output: 4069
🏁 Script executed:
#!/bin/bash
# Check what FCPath does with None - search for its documentation or test
web_search "filecache FCPath None TypeError"Repository: SETI/rms-metadata-tools
Length of output: 117
🏁 Script executed:
#!/bin/bash
# Check if there are any other Suite instantiations
rg -n "Suite(" metadata_tools/geometry_support.pyRepository: SETI/rms-metadata-tools
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Search for util.select_dir to understand what it returns
rg -n "def select_dir" metadata_tools/ --type=py -A 10Repository: SETI/rms-metadata-tools
Length of output: 631
🏁 Script executed:
#!/bin/bash
# Fix the Suite regex search - escape the parenthesis
rg -n "Suite\\\(" metadata_tools/geometry_support.pyRepository: SETI/rms-metadata-tools
Length of output: 133
🏁 Script executed:
#!/bin/bash
# Check the documentation for FCPath from filecache package
# See if we can find any info about its None handling
python3 << 'EOF'
import sys
try:
from filecache import FCPath
# Test if FCPath(None) raises an error
try:
result = FCPath(None)
print(f"FCPath(None) succeeded: {result}")
except TypeError as e:
print(f"FCPath(None) raises TypeError: {e}")
except Exception as e:
print(f"FCPath(None) raises {type(e).__name__}: {e}")
except ImportError:
print("filecache not installed in test environment")
EOFRepository: SETI/rms-metadata-tools
Length of output: 110
🏁 Script executed:
#!/bin/bash
# Search for Suite instantiations without regex special chars
rg -n "Suite" metadata_tools/geometry_support.py | grep "="Repository: SETI/rms-metadata-tools
Length of output: 533
🏁 Script executed:
#!/bin/bash
# Look at the full select_dir function to understand if it can return None
sed -n '31,50p' metadata_tools/util.pyRepository: SETI/rms-metadata-tools
Length of output: 606
🏁 Script executed:
#!/bin/bash
# Check if metadata_dir is ever None in the codebase - search for any guards
rg -n "metadata_dir" metadata_tools/ --type=py | grep -i "none\|if\|is "Repository: SETI/rms-metadata-tools
Length of output: 316
🏁 Script executed:
#!/bin/bash
# Let's examine the Suite.__init__ more carefully and its usage of metadata_dir
sed -n '1237,1280p' metadata_tools/geometry_support.pyRepository: SETI/rms-metadata-tools
Length of output: 1925
🏁 Script executed:
#!/bin/bash
# Check index_support.py Suite class to see if it has the same pattern
sed -n '26,80p' metadata_tools/index_support.pyRepository: SETI/rms-metadata-tools
Length of output: 2473
🏁 Script executed:
#!/bin/bash
# Search for any conditional checks on metadata_dir in the codebase
rg -n "if.*metadata_dir" metadata_tools/ --type=py -B 2 -A 2Repository: SETI/rms-metadata-tools
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Look for other similar Suite classes or similar patterns with optional path parameters
rg -n "FCPath\(.*=None" metadata_tools/ --type=pyRepository: SETI/rms-metadata-tools
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or tests about FCPath behavior
find . -name "*test*" -o -name "*example*" | grep -E "\.(py|md)$" | head -10Repository: SETI/rms-metadata-tools
Length of output: 248
🏁 Script executed:
#!/bin/bash
# Check the test files to see how Suite and metadata_dir are used
cat metadata_tools/hosts/GO_0xxx/tests/test_geometry.py | head -100Repository: SETI/rms-metadata-tools
Length of output: 3894
🏁 Script executed:
#!/bin/bash
# Look for Suite instantiation in tests
rg -n "Suite" ./tests/test_geometry.py -B 2 -A 2Repository: SETI/rms-metadata-tools
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check if there are any other geometry or index class instantiations with None values
rg -n "Suite\|IndexTable" ./tests/ --type=py -B 1 -A 1Repository: SETI/rms-metadata-tools
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for any documentation about FCPath in README or comments
find . -name "README*" -o -name "*.md" | xargs rg -l "FCPath" 2>/dev/null | head -5Repository: SETI/rms-metadata-tools
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Let's trace back to see if metadata_tree could be None
rg -n "metadata_tree" metadata_tools/geometry_support.py | head -20Repository: SETI/rms-metadata-tools
Length of output: 196
🏁 Script executed:
#!/bin/bash
# Check what args.metadata_tree is and if it's required
rg -n "metadata_tree" metadata_tools/geometry_support.py -B 2 -A 2 | head -40Repository: SETI/rms-metadata-tools
Length of output: 524
🏁 Script executed:
#!/bin/bash
# Search for the argument parser to see if metadata_tree is required
rg -n "add_argument.*metadata" metadata_tools/ --type=py -B 1 -A 3Repository: SETI/rms-metadata-tools
Length of output: 530
🏁 Script executed:
#!/bin/bash
# Check if IndexTable also has the same issue with FCPath(None) being called
sed -n '26,60p' metadata_tools/index_support.pyRepository: SETI/rms-metadata-tools
Length of output: 1549
🏁 Script executed:
#!/bin/bash
# Check what happens if IndexTable is instantiated with None metadata_dir
# Look for where IndexTable is instantiated
rg -n "IndexTable(" metadata_tools/ --type=py -B 1 -A 1Repository: SETI/rms-metadata-tools
Length of output: 139
🏁 Script executed:
#!/bin/bash
# Check the early return guard in IndexTable more carefully
sed -n '26,52p' metadata_tools/index_support.pyRepository: SETI/rms-metadata-tools
Length of output: 1342
🏁 Script executed:
#!/bin/bash
# Verify if select_dir can ever return None
sed -n '31,50p' metadata_tools/util.pyRepository: SETI/rms-metadata-tools
Length of output: 606
🏁 Script executed:
#!/bin/bash
# Check if there's any scenario where Suite could be called without metadata_dir
rg -n "def create" metadata_tools/geometry_support.py -B 3 -A 8 | head -50Repository: SETI/rms-metadata-tools
Length of output: 600
🏁 Script executed:
#!/bin/bash
# Check if there's any usage pattern where Suite is created with None for metadata_dir
rg -n "Suite(" metadata_tools/ tests/ --type=py -A 3 | grep -v "Suite.get" | head -50Repository: SETI/rms-metadata-tools
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Let's check what all the parameters do in __init__ to understand if metadata_dir is truly optional
sed -n '1237,1290p' metadata_tools/geometry_support.pyRepository: SETI/rms-metadata-tools
Length of output: 2408
🏁 Script executed:
#!/bin/bash
# Check if metadata_dir usage can be conditional
rg -n "self.metadata_dir" metadata_tools/geometry_support.py -B 1 -A 1Repository: SETI/rms-metadata-tools
Length of output: 393
🏁 Script executed:
#!/bin/bash
# Verify by checking if there's any path in the code where Suite init completes with None metadata_dir
# Look for any scenario where the glob parameter could be None (which would skip the metadata_dir.glob call)
sed -n '1260,1285p' metadata_tools/geometry_support.pyRepository: SETI/rms-metadata-tools
Length of output: 1064
🏁 Script executed:
#!/bin/bash
# Search for all instantiations of Suite to verify metadata_dir is always provided
rg -n "Suite\s*=" metadata_tools/ tests/ --type=py -A 2 -B 1Repository: SETI/rms-metadata-tools
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Double-check line 1646 and surrounding context to verify metadata_dir is always provided
sed -n '1620,1655p' metadata_tools/geometry_support.pyRepository: SETI/rms-metadata-tools
Length of output: 1543
🏁 Script executed:
#!/bin/bash
# Check if index_glob could be None, which might affect whether metadata_dir.glob() is called
rg -n "index_glob" metadata_tools/geometry_support.py | grep -v "self.index_glob" | head -10Repository: SETI/rms-metadata-tools
Length of output: 448
🏁 Script executed:
#!/bin/bash
# Look for any other places where Suite.__init__ might be called
find . -name "*.py" -type f -exec grep -l "Suite" {} \; | xargs grep -n "Suite" | grep -v "class Suite" | grep -v "test_"Repository: SETI/rms-metadata-tools
Length of output: 609
🏁 Script executed:
#!/bin/bash
# Final check: look for any other files that might import and use Suite
find . -name "*.py" -type f | xargs grep -l "from.*geometry_support\|import.*Suite" | grep -v test | grep -v __pycache__Repository: SETI/rms-metadata-tools
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Check if there are examples or any documentation showing Suite usage
find . -name "*.md" -o -name "*.rst" -o -name "*.txt" | xargs grep -l "Suite" 2>/dev/null | head -5Repository: SETI/rms-metadata-tools
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check the GO_0xxx_geometry_cloud.py file for Suite usage
cat ./metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py | head -200Repository: SETI/rms-metadata-tools
Length of output: 3008
🏁 Script executed:
#!/bin/bash
# Search for Suite usage in that file
rg -n "Suite" ./metadata_tools/hosts/GO_0xxx/GO_0xxx_geometry_cloud.py -B 2 -A 2Repository: SETI/rms-metadata-tools
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Let's check if there's any documentation about what filecache.FCPath does
# and verify whether it can handle None
rg -n "filecache" . --type=pyRepository: SETI/rms-metadata-tools
Length of output: 768
🏁 Script executed:
#!/bin/bash
# Check if there are any requirements or setup files that list filecache version
find . -name "requirements*.txt" -o -name "setup.py" -o -name "pyproject.toml" | xargs cat 2>/dev/null | grep -i filecacheRepository: SETI/rms-metadata-tools
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Search for any guarding or conditional access to metadata_dir in Suite methods
rg -n "metadata_dir" metadata_tools/geometry_support.py -B 1 -A 1Repository: SETI/rms-metadata-tools
Length of output: 1032
🏁 Script executed:
#!/bin/bash
# Check if there's any initialization or check that would prevent the None issue
sed -n '1237,1300p' metadata_tools/geometry_support.pyRepository: SETI/rms-metadata-tools
Length of output: 2779
🏁 Script executed:
#!/bin/bash
# Final verification: confirm rms-filecache behavior with None by checking documentation or tests
# Look for any test files that might show FCPath behavior
find . -name "*test*" -type f -name "*.py" | xargs grep -l "FCPath" 2>/dev/null | head -5Repository: SETI/rms-metadata-tools
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check if there are any comments or documentation about FCPath(None) behavior
rg -n "FCPath\(" metadata_tools/ --type=py -B 1 | grep -i "none\|optional\|guard" | head -20Repository: SETI/rms-metadata-tools
Length of output: 136
Add defensive check for metadata_dir parameter before calling FCPath().
The metadata_dir parameter defaults to None, but line 1259 calls FCPath(metadata_dir) unconditionally. This will raise a TypeError at runtime if metadata_dir is None. While the current call site at line 1646 always provides metadata_dir via util.select_dir(), the optional parameter signature creates a contract violation—allowing None suggests it's optional, but the code requires it. Add a defensive check or remove the default value to clarify the API contract.
🔧 Suggested fix
- self.metadata_dir = FCPath(metadata_dir)
+ if metadata_dir is None:
+ raise ValueError("metadata_dir is required for geometry processing")
+ self.metadata_dir = FCPath(metadata_dir)🧰 Tools
🪛 Flake8 (7.3.0)
[error] 1238-1238: continuation line over-indented for visual indent
(E127)
🪛 Ruff (0.14.11)
1237-1237: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
🤖 Prompt for AI Agents
In `@metadata_tools/geometry_support.py` around lines 1237 - 1261, In the __init__
constructor of the geometry Suite class the code unconditionally calls
FCPath(metadata_dir) even though metadata_dir defaults to None; change this to
defensively handle None by only wrapping metadata_dir with FCPath when it's not
None (e.g., if metadata_dir is None set self.metadata_dir = None else
self.metadata_dir = FCPath(metadata_dir)), or alternatively remove the None
default and require metadata_dir as a non-optional parameter—update the
assignment to self.metadata_dir and adjust the signature/docstring accordingly;
refer to the __init__ method and the FCPath and metadata_dir symbols to locate
the fix.
| #!/bin/bash | ||
| apt-get update -y | ||
|
|
||
| apt-get install -y python3 python3-pip python3-venv git | ||
| cd || exit 1 | ||
| git clone https://github.com/SETI/rms-metadata-tools.gitnew_overrides | ||
| cd rms-metadata-tools || exit 1 |
There was a problem hiding this comment.
Critical: Malformed git clone URL.
Line 6 has an invalid repository URL: https://github.com/SETI/rms-metadata-tools.gitnew_overrides. The URL should end with .git, not .gitnew_overrides. This appears to be a merge artifact or copy-paste error that will cause the git clone to fail.
🐛 Proposed fix
cd || exit 1
-git clone https://github.com/SETI/rms-metadata-tools.gitnew_overrides
+git clone https://github.com/SETI/rms-metadata-tools.git || exit 1
cd rms-metadata-tools || exit 1🤖 Prompt for AI Agents
In `@metadata_tools/hosts/GO_0xxx/gcp_index_startup.sh` around lines 1 - 7, The
git clone URL in the startup script is malformed: replace the incorrect
repository string "https://github.com/SETI/rms-metadata-tools.gitnew_overrides"
used in the git clone command with the correct URL
"https://github.com/SETI/rms-metadata-tools.git" (and if you intended to use the
new_overrides branch, add an explicit checkout of that branch after cloning,
e.g. run git checkout new_overrides in the rms-metadata-tools repo).
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.