Skip to content

Commit 9883739

Browse files
VisLabVisLab
committed
Corrected the copilot suggestions
1 parent 5e65c6c commit 9883739

4 files changed

Lines changed: 82 additions & 55 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ The HED ecosystem consists of several interconnected repositories:
165165
| [hed-specification](https://github.com/hed-standard/hed-specification) | Official HED specification documents |
166166
| [hed-schemas](https://github.com/hed-standard/hed-schemas) | Official HED schema repository |
167167
| [ndx-hed](https://github.com/hed-standard/ndx-hed) | HED support for NWB |
168-
| [hed-javascript](https://github.com/hed-standard/hed-javascript) | HED in BIDS |
168+
| [hed-javascript](https://github.com/hed-standard/hed-javascript) | JavaScript HEd validation tools |
169169

170170

171171
## Contributing

‎hed/cli/cli.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
12
#!/usr/bin/env python3
23
"""
34
HED Command Line Interface
@@ -95,7 +96,7 @@ def validate_bids_cmd(
9596
if error_limit is not None:
9697
args.extend(["-ec", str(error_limit)])
9798
if errors_by_file:
98-
args.extend(["-ef", "True"])
99+
args.append("-ef")
99100
if format:
100101
args.extend(["-f", format])
101102
if log_level:

‎hed/scripts/validate_bids.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,9 @@ def get_parser():
3939
)
4040
parser.add_argument(
4141
"-ef",
42-
"--errors_by_file",
42+
"--errors-by-file",
43+
action="store_true",
4344
dest="errors_by_file",
44-
type=bool,
45-
default=False,
4645
help="Apply error limit by file rather than overall for text output.",
4746
)
4847
parser.add_argument(

‎tests/test_cli_parameter_parity.py‎

Lines changed: 77 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -155,19 +155,26 @@ def test_remodel_backup_parameters(self):
155155

156156
cli_opts = self._get_click_options(backup_command)
157157

158-
# Check positional (data_dir)
159-
self.assertEqual(len(cli_opts["positional"]), 1, "Should have 1 positional argument (data_dir)")
160-
self.assertEqual(cli_opts["positional"][0], "data_dir", "Positional argument should be data_dir")
158+
# Check positional count matches
159+
self.assertEqual(
160+
len(cli_opts["positional"]),
161+
len(original_opts["positional"]),
162+
f"Positional argument count mismatch: CLI has {len(cli_opts['positional'])}, original has {len(original_opts['positional'])}",
163+
)
161164

162-
# Check key parameters exist
165+
# Check optional parameters from original parser exist in CLI
166+
original_dests = set(original_opts["optional"].keys())
163167
cli_dests = set(cli_opts["optional"].keys())
164-
required_params = ["backup_dir", "backup_name", "suffixes", "task_names", "exclude_dirs"]
165-
for param in required_params:
166-
self.assertIn(param, cli_dests, f"Parameter '{param}' not found in backup CLI command")
168+
169+
for orig_dest in original_dests:
170+
self.assertIn(orig_dest, cli_dests, f"Parameter '{orig_dest}' from original parser not found in CLI")
167171

168-
# Check verbose flag
172+
# Check flags from original parser exist in CLI
173+
original_flags = {flag[0] for flag in original_opts["flags"]}
169174
cli_flags = {flag[0] for flag in cli_opts["flags"]}
170-
self.assertIn("verbose", cli_flags, "verbose flag not found in backup CLI command")
175+
176+
for orig_flag in original_flags:
177+
self.assertIn(orig_flag, cli_flags, f"Flag '{orig_flag}' from original parser not found in CLI")
171178

172179
def test_remodel_restore_parameters(self):
173180
"""Test remodel restore CLI parameters match run_remodel_restore.py parser."""
@@ -180,14 +187,26 @@ def test_remodel_restore_parameters(self):
180187

181188
cli_opts = self._get_click_options(restore_command)
182189

183-
# Check positional
184-
self.assertEqual(cli_opts["positional"][0], "data_dir", "Positional argument should be data_dir")
190+
# Check positional count matches
191+
self.assertEqual(
192+
len(cli_opts["positional"]),
193+
len(original_opts["positional"]),
194+
f"Positional argument count mismatch: CLI has {len(cli_opts['positional'])}, original has {len(original_opts['positional'])}",
195+
)
185196

186-
# Check key parameters
197+
# Check optional parameters from original parser exist in CLI
198+
original_dests = set(original_opts["optional"].keys())
187199
cli_dests = set(cli_opts["optional"].keys())
188-
required_params = ["backup_dir", "backup_name", "task_names"]
189-
for param in required_params:
190-
self.assertIn(param, cli_dests, f"Parameter '{param}' not found in restore CLI command")
200+
201+
for orig_dest in original_dests:
202+
self.assertIn(orig_dest, cli_dests, f"Parameter '{orig_dest}' from original parser not found in CLI")
203+
204+
# Check flags from original parser exist in CLI
205+
original_flags = {flag[0] for flag in original_opts["flags"]}
206+
cli_flags = {flag[0] for flag in cli_opts["flags"]}
207+
208+
for orig_flag in original_flags:
209+
self.assertIn(orig_flag, cli_flags, f"Flag '{orig_flag}' from original parser not found in CLI")
191210

192211
def test_remodel_run_parameters(self):
193212
"""Test remodel run CLI parameters match run_remodel.py parser."""
@@ -200,36 +219,26 @@ def test_remodel_run_parameters(self):
200219

201220
cli_opts = self._get_click_options(run_command)
202221

203-
# Check positional arguments (data_dir and model_path)
222+
# Check positional count matches
204223
self.assertEqual(
205-
len(cli_opts["positional"]), 2, f"Should have 2 positional arguments, got {len(cli_opts['positional'])}"
224+
len(cli_opts["positional"]),
225+
len(original_opts["positional"]),
226+
f"Positional argument count mismatch: CLI has {len(cli_opts['positional'])}, original has {len(original_opts['positional'])}",
206227
)
207-
self.assertEqual(cli_opts["positional"][0], "data_dir", "First positional should be data_dir")
208-
self.assertEqual(cli_opts["positional"][1], "model_path", "Second positional should be model_path")
209228

210-
# Check key optional parameters exist
229+
# Check optional parameters from original parser exist in CLI
230+
original_dests = set(original_opts["optional"].keys())
211231
cli_dests = set(cli_opts["optional"].keys())
212-
required_params = [
213-
"backup_dir",
214-
"backup_name",
215-
"suffixes",
216-
"individual_summaries",
217-
"json_sidecar",
218-
"log_dir",
219-
"hed_versions",
220-
"save_formats",
221-
"task_names",
222-
"work_dir",
223-
"exclude_dirs",
224-
]
225-
for param in required_params:
226-
self.assertIn(param, cli_dests, f"Parameter '{param}' not found in remodel run CLI command")
232+
233+
for orig_dest in original_dests:
234+
self.assertIn(orig_dest, cli_dests, f"Parameter '{orig_dest}' from original parser not found in CLI")
227235

228-
# Check flags
236+
# Check flags from original parser exist in CLI
237+
original_flags = {flag[0] for flag in original_opts["flags"]}
229238
cli_flags = {flag[0] for flag in cli_opts["flags"]}
230-
required_flags = ["use_bids", "no_backup", "no_summaries", "no_update", "verbose"]
231-
for flag in required_flags:
232-
self.assertIn(flag, cli_flags, f"Flag '{flag}' not found in remodel run CLI command")
239+
240+
for orig_flag in original_flags:
241+
self.assertIn(orig_flag, cli_flags, f"Flag '{orig_flag}' from original parser not found in CLI")
233242

234243
def test_extract_sidecar_parameters(self):
235244
"""Test extract-sidecar CLI parameters match hed_extract_bids_sidecar.py parser."""
@@ -241,14 +250,26 @@ def test_extract_sidecar_parameters(self):
241250

242251
cli_opts = self._get_click_options(extract_command)
243252

244-
# Check positional
245-
self.assertEqual(cli_opts["positional"][0], "data_path", "Positional should be data_path")
253+
# Check positional count matches
254+
self.assertEqual(
255+
len(cli_opts["positional"]),
256+
len(original_opts["positional"]),
257+
f"Positional argument count mismatch: CLI has {len(cli_opts['positional'])}, original has {len(original_opts['positional'])}",
258+
)
246259

247-
# Check key parameters
260+
# Check optional parameters from original parser exist in CLI
261+
original_dests = set(original_opts["optional"].keys())
248262
cli_dests = set(cli_opts["optional"].keys())
249-
required_params = ["suffix", "value_columns", "skip_columns", "log_level", "log_file", "output_file", "exclude_dirs"]
250-
for param in required_params:
251-
self.assertIn(param, cli_dests, f"Parameter '{param}' not found in extract-sidecar CLI command")
263+
264+
for orig_dest in original_dests:
265+
self.assertIn(orig_dest, cli_dests, f"Parameter '{orig_dest}' from original parser not found in CLI")
266+
267+
# Check flags from original parser exist in CLI
268+
original_flags = {flag[0] for flag in original_opts["flags"]}
269+
cli_flags = {flag[0] for flag in cli_opts["flags"]}
270+
271+
for orig_flag in original_flags:
272+
self.assertIn(orig_flag, cli_flags, f"Flag '{orig_flag}' from original parser not found in CLI")
252273

253274
def test_schema_validate_parameters(self):
254275
"""Test schema validate CLI parameters match validate_schemas.py parser."""
@@ -261,13 +282,19 @@ def test_schema_validate_parameters(self):
261282

262283
cli_opts = self._get_click_options(validate_command)
263284

264-
# Check that schema_path positional exists
265-
self.assertIn("schema_path", cli_opts["positional"], "schema_path positional argument not found")
285+
# Check positional count matches (both should have 1)
286+
self.assertEqual(
287+
len(cli_opts["positional"]),
288+
len(original_opts["positional"]),
289+
f"Positional argument count mismatch: CLI has {len(cli_opts['positional'])}, original has {len(original_opts['positional'])}",
290+
)
266291

267-
# Check flags
292+
# Check flags from original parser exist in CLI
293+
original_flags = {flag[0] for flag in original_opts["flags"]}
268294
cli_flags = {flag[0] for flag in cli_opts["flags"]}
269-
self.assertIn("add_all_extensions", cli_flags, "add_all_extensions flag not found")
270-
self.assertIn("verbose", cli_flags, "verbose flag not found")
295+
296+
for orig_flag in original_flags:
297+
self.assertIn(orig_flag, cli_flags, f"Flag '{orig_flag}' from original parser not found in CLI")
271298

272299
def test_schema_add_ids_parameters(self):
273300
"""Test schema add-ids uses positional arguments."""

0 commit comments

Comments
 (0)