Support ordering circuits by target_simulator#640
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
Author
pgetta
changed the title
Contributor
Author
|
Thank you Eleftherios. |
2 tasks
GianlucaFicarelli
approved these changes
Jun 24, 2026
Add target_simulator to CircuitFilter ordering_model_fields so GET /circuit accepts order_by=target_simulator (ascending/descending) instead of returning a 422. Reorder the TargetSimulator enum to alphabetical order so the native Postgres enum sort order is intuitive, with a migration to sync the DB enum values. Add a circuit ordering test (the suite previously had none) that exercises every sortable field and spot-checks target_simulator order via the enum's definition order.
pgetta
force-pushed
the
feat/order-circuits-by-target-simulator
branch
from
f44e576 to
98f5801
Compare
Contributor
Author
|
Had to rebase and force push due to a conflicting migrations. Could you please re-approve? |
Contributor
Author
|
Ah, I thought due to the rebase I've lost the approvals, but that's not the case. Merging now. |
This was referenced Jun 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enables ordering the
GET /circuitlist endpoint bytarget_simulator, which previously returned a422because the field was not whitelisted for sorting.Changes
app/filters/circuit.py— addtarget_simulatortoCircuitFilter.Constants.ordering_model_fields, soorder_by=target_simulator/order_by=-target_simulatorare accepted. (target_simulatorwas already a supported filter; this adds sorting.)app/db/types.py— reorder theTargetSimulatorenum into alphabetical order (Brian2, CORENEURON, LearningEngine, NEURON). Native Postgres enums sort by definition order, so this makes the ascending sort intuitive and lexicographic.alembic/versions/…9e36650e03a9…— migration syncing the DB enum to the new order viaop.sync_enum_values, with a symmetric downgrade. Thecircuit.target_simulatorcolumn and its'NEURON'server default are preserved.tests/test_circuit.py— circuits had no ordering test; addtest_orderingthat exercises every sortable field (asc/desc, count-preserving) and spot-checkstarget_simulatororder against the enum's definition order. Also variestarget_simulatoracross themodelsfixture and updates the affected filter assertion.scripts/export/build_database_archive.sh— refreshed head revision (auto-updated bymake migration).Deployment note
sync_enum_valuescan't reorder a Postgres enum in place — it recreates the type and runsALTER TABLE circuit ALTER COLUMN … TYPE … USING …, which takes anACCESS EXCLUSIVElock and rewrites the column for the duration. No data is at risk (no values removed/renamed), but it briefly blocks on a largecircuittable.Testing
make lint— ruff + pyright clean.PYTEST_ADDOPTS="-k test_circuit" make test-local— 52 passed, including the newtest_orderingand the updatedtest_filtering, against a DB with the new migration applied.