Add expression interpreter - #71
Conversation
4229095 to
19cdfd0
Compare
19cdfd0 to
5bda559
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
==========================================
+ Coverage 90.56% 90.72% +0.16%
==========================================
Files 13 15 +2
Lines 890 1121 +231
Branches 130 194 +64
==========================================
+ Hits 806 1017 +211
- Misses 50 61 +11
- Partials 34 43 +9 🚀 New features to boost your workflow:
|
=== Do not change lines below ===
{
"chain": [],
"cmd": "uv lock",
"exit": 0,
"extra_inputs": [],
"inputs": [
"pyproject.toml",
"uv.lock"
],
"outputs": [
"uv.lock"
],
"pwd": "."
}
^^^ Do not change lines above ^^^
=== Do not change lines below ===
{
"chain": [],
"cmd": "uv lock",
"exit": 0,
"extra_inputs": [],
"inputs": [
"pyproject.toml",
"uv.lock"
],
"outputs": [
"uv.lock"
],
"pwd": "."
}
^^^ Do not change lines above ^^^
|
From today's call, here's a quick-and-dirty proxy to allow you to provide multiple namespaces and treat them as one: class LookupProxy:
def __init__(self, *objs):
self.objs = objs
def __getitem__(self, key):
for obj in self.objs:
if isinstance(obj, dict) and key in obj:
return obj[key]
if hasattr(obj, key):
return getattr(obj, key)
raise AttributeError
# Provide either attr or item lookup
__getattr__ = __getitem__Example usage: import bids_validator as bv
import bids_validator.context
from bids_validator import expression_language as el
from bids_validator.types.files import FileTree
from bidsschematools.schema import load_schema
schema = load_schema()
root = FileTree.read_from_filesystem('tests/data/bids-examples/ds000117/')
ds = bv.context.Dataset(root, schema)
file = root / 'sub-01' / 'ses-mri' / 'anat' / 'sub-01_ses-mri_acq-mprage_T1w.nii.gz'
context = bv.context.Context(file, ds, None)
namespace = LookupProxy(el.ids, context) |
Primary change is in Element case to handle nulls, changes in Function case are more for my understanding
|
The |
|
Needs further work:
For future work:
|
|
Okay, I have created a function for |
|
I think as a method on the context is probably simplest. The alternative would be as a function that takes a context as an argument, and then the proxy setup uses |
|
Yep that works, and the evaluator can find and use it from the namespace produced by the |
effigies
left a comment
effigies
left a comment
There was a problem hiding this comment.
Looking good. A few notes.
To match the TS version
|
|
There was a problem hiding this comment.
Pull request overview
Adds an initial implementation of the BIDS schema expression language interpreter and extends Context with an exists(...) helper used by schema rules, along with tests and dependency updates to support expression parsing.
Changes:
- Introduce
src/bids_validator/expression_language.pyimplementing expression parsing + evaluation with a small built-in function/value namespace. - Add
Context.exists(...)supporting dataset/stimuli/file/subject/bids-uri existence checks, with new tests intests/test_context.py. - Update
pyproject.tomlto installbidsschematoolswith theexpressionsextra; add ademodependency group.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| tests/test_context.py | Adds coverage for Context.exists(...) across multiple rule modes. |
| src/bids_validator/expression_language.py | New expression interpreter (AST evaluation + built-in functions). |
| src/bids_validator/context.py | Adds Context.exists(...) implementation including bids-uri handling. |
| pyproject.toml | Enables bidsschematools expressions extra; adds demo dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| match var: | ||
| case bool(): | ||
| return 'boolean' | ||
| case int() | float(): | ||
| return 'number' | ||
| case list(): | ||
| return 'array' | ||
| case dict(): | ||
| return 'object' | ||
| case _: | ||
| return 'null' |
| def __getitem__(self, key): | ||
| for obj in self.objs: | ||
| if isinstance(obj, dict) and key in obj: | ||
| return obj[key] | ||
| if hasattr(obj, key): | ||
| return getattr(obj, key) | ||
| raise AttributeError |
| def interpret(rule: str, context: Context) -> Any: | ||
| """Interpret a rule from the schema in a given file context""" | ||
| namespace = LookupProxy(el_namespace, context) | ||
| expr = bst_expr.parse(rule) | ||
| return evaluate(expr, namespace) |
| if isinstance(arg, (float, int)): | ||
| return arg | ||
|
|
||
| return max(filter_strs(arg)) |
| if isinstance(arg, (float, int)): | ||
| return arg | ||
|
|
||
| return min(filter_strs(arg)) |
Uses tests from schema.meta.expression_tests
|
Test for the expression language has been added, but I'm not 100% happy about having to import the schema again. It was either this or looping through the expression tests in the test, but that treats it as a single test, and so doesn't give info on which expression has failed. |
9b3675a to
2aa437f
Compare
Here's the initial attempt at adding all the parts of the expression language interpreter.
These successfully cover the full range of tests in the
bidsschematoolsexpression tests.Two things that still need addressing:
existspasses the tests but needs more work to actually work as described in the schema tools docsContext