Skip to content

ORM defaults and integration test#1429

Open
MaximeBICMTL wants to merge 1 commit into
aces:bids_staging_branchfrom
MaximeBICMTL:orm-defaults
Open

ORM defaults and integration test#1429
MaximeBICMTL wants to merge 1 commit into
aces:bids_staging_branchfrom
MaximeBICMTL:orm-defaults

Conversation

@MaximeBICMTL

@MaximeBICMTL MaximeBICMTL commented May 25, 2026

Copy link
Copy Markdown
Contributor

Description

This PR adds default values to the ORM columns based on the values specified in the LORIS SQL schema.

The default values are added to the Python side, which has two advantages:

  • First, this allows to use the decorators specified in the models (example: use True instead of "Y")
  • Second, this allows to use the field values directly upon model object construction, rather than after flushing to the database. This also allows to have better typing down the line (since fields are always populated, they do not need to be nullable).

Changes

This PR contains three changes:

  1. Improve the test_orm_sql_sync.py integration test to compare ORM defaults to those of the LORIS SQL schema (non-trivial since ORM defaults are compared to SQL ones).
  2. Update the ORM models using the updated integration test.
  3. Remove some initialization values that were specified manually in the code but are no longer needed since they are now specified as default.

Note

Note that the actual default values are very often bad and potentially harmful IMO, but this would need to be corrected in the LORIS SQL schema rather than in this PR.

@github-actions github-actions Bot added Language: Python Issue or PR related to the Python codebase Package: DICOM importer PR or issue related to the DICOM importer labels May 25, 2026
@MaximeBICMTL MaximeBICMTL added Area: CI Issue or PR related to continuous integration, including automated tests and static checks Area: ORM Issue or PR related to the SQLAlchemy integration Complexity: Medium Issue or PR that requires a moderate effort or expertise to implement, review, or test and removed Package: DICOM importer PR or issue related to the DICOM importer labels May 25, 2026
@github-actions github-actions Bot added the Package: DICOM importer PR or issue related to the DICOM importer label May 25, 2026
@MaximeBICMTL MaximeBICMTL marked this pull request as ready for review May 25, 2026 11:08
@MaximeBICMTL MaximeBICMTL removed the Package: DICOM importer PR or issue related to the DICOM importer label May 25, 2026
@MaximeBICMTL MaximeBICMTL changed the base branch from main to bids_staging_branch June 3, 2026 03:35
@github-actions github-actions Bot added the Package: DICOM importer PR or issue related to the DICOM importer label Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: CI Issue or PR related to continuous integration, including automated tests and static checks Area: ORM Issue or PR related to the SQLAlchemy integration Complexity: Medium Issue or PR that requires a moderate effort or expertise to implement, review, or test Language: Python Issue or PR related to the Python codebase Package: DICOM importer PR or issue related to the DICOM importer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant