fzl impl, and better handling of multitypes args (calculators, model, ...) #58
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a new fzl command for listing models and calculators with enhanced pattern matching capabilities, and refactors calculator/model argument handling for better modularity and maintainability.
Key Changes:
- Added new
fzlfunction and CLI command that lists models and calculators with support for glob/regex patterns, model-calculator compatibility checking, and optional validation - Moved calculator-related helper functions from
fz/core.pytofz/helpers.pyfor better code organization - Implemented generic item resolution functions that support multiple argument forms: plain URIs, JSON files/strings, aliases, glob patterns, and regex patterns
- Replaced the old
listcommand implementation with the newfzlfunctionality (breaking change: removed--globalflag in favor of--modelsand--calculatorspattern matching)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_cli_commands.py |
Removed two tests for the old list command's --global flag functionality |
tests/test_calculator_discovery.py |
Added 600+ lines of comprehensive tests covering calculator argument forms, glob patterns, regex patterns, JSON file patterns, and Windows-specific calculator handling |
setup.py |
Added fzl entry point to console scripts |
fz/helpers.py |
Moved calculator functions from core.py; added new generic item resolution functions supporting multiple argument forms and pattern matching |
fz/core.py |
Removed calculator helper functions (moved to helpers.py); added new fzl function for listing models/calculators with validation support |
fz/cli.py |
Added fzl_main CLI entry point with markdown/JSON/table output formats; updated list command to use new fzl functionality |
fz/__init__.py |
Exported new fzl function in public API |
Comments suppressed due to low confidence (2)
fz/helpers.py:1964
- Except block directly handles BaseException.
except:
fz/helpers.py:1994
- Except block directly handles BaseException.
except:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| parsed = _parse_argument(item_str, alias_type=item_type) | ||
| if isinstance(parsed, dict): | ||
| if item_type == 'calculators': | ||
|
|
There was a problem hiding this comment.
This blank line has incorrect indentation. Remove this blank line or fix its indentation.
| if not _calculator_supports_model(item_data, model_name): | ||
| continue | ||
|
|
||
|
|
There was a problem hiding this comment.
Extra blank lines with incorrect indentation. Remove these blank lines or fix their indentation to be consistent with the surrounding code.
| if not _calculator_supports_model(item_data, model_name): | ||
| continue | ||
|
|
||
|
|
There was a problem hiding this comment.
Extra blank lines with incorrect indentation. Remove these blank lines or fix their indentation to be consistent with the surrounding code.
| except: | ||
| pass |
There was a problem hiding this comment.
Bare except: clause catches all exceptions including KeyboardInterrupt and SystemExit, which can make debugging difficult. Specify the exception type(s) you want to catch, such as except (ValueError, TypeError, KeyError): or at minimum use except Exception:.
| for item in parsed: | ||
| if isinstance(item, dict): | ||
| if item_type == 'calculators': | ||
|
|
There was a problem hiding this comment.
This blank line has incorrect indentation (appears to have extra leading whitespace). Remove this blank line or fix its indentation.
| parsed = _parse_argument(item_str, alias_type=item_type) | ||
| if isinstance(parsed, dict): | ||
| if item_type == 'calculators': | ||
|
|
There was a problem hiding this comment.
This blank line has incorrect indentation. Remove this blank line or fix its indentation.
| for item in parsed: | ||
| if isinstance(item, dict): | ||
| if item_type == 'calculators': | ||
|
|
There was a problem hiding this comment.
This blank line has incorrect indentation. Remove this blank line or fix its indentation.
| _calculator_supports_model, | ||
| _extract_calculator_uri, | ||
| _find_all_calculators, | ||
| _filter_calculators_by_model, |
There was a problem hiding this comment.
Import of '_calculator_supports_model' is not used.
Import of '_extract_calculator_uri' is not used.
Import of '_find_all_calculators' is not used.
Import of '_filter_calculators_by_model' is not used.
| _calculator_supports_model, | |
| _extract_calculator_uri, | |
| _find_all_calculators, | |
| _filter_calculators_by_model, |
| result = fzl_func(models=args.models, calculators=args.calculators, check=args.check) | ||
|
|
||
| if args.format == "json": | ||
| import json |
| result = fzl_func(models=args.models, calculators=args.calculators, check=args.check) | ||
|
|
||
| if args.format == "json": | ||
| import json |
No description provided.