Skip to content

Conversation

@yarikoptic
Copy link
Collaborator

Originally it was pinned this way in
17db824
but no clear indication was recorded on why that is needed. If sqlalchemy really follows semver then it should remain comaptible with all the way to 2.0 .

Otherwise - it makes pybids not installable in an env which actually requires newer sqlalchemy (more plausing) like in our case see datalad/datalad-registry#141 (comment)

Originally it was pinned this way in
17db824
but no clear indication was recorded on why that is needed.
If sqlalchemy really follows semver then it should remain
comaptible with all the way to 2.0 .

Otherwise - it makes pybids not installable in an env which
actually requires newer sqlalchemy (more plausing) like in our case
see datalad/datalad-registry#141 (comment)
@effigies
Copy link
Collaborator

effigies commented Mar 23, 2023

This will break. We rely on deprecated behavior that was removed in 1.4.

xref #679

@codecov
Copy link

codecov bot commented Mar 23, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.19 🎉

Comparison is base (18778c1) 86.35% compared to head (2b47f62) 86.54%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #959      +/-   ##
==========================================
+ Coverage   86.35%   86.54%   +0.19%     
==========================================
  Files          35       35              
  Lines        4023     4043      +20     
  Branches      974      974              
==========================================
+ Hits         3474     3499      +25     
+ Misses        355      353       -2     
+ Partials      194      191       -3     

see 6 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yarikoptic
Copy link
Collaborator Author

sample failed tests from a pre-release (ubuntu-latest, 3.8, pip, ci_tests, PRE_PIP_FLAGS)

FAILED bids/layout/tests/test_layout_on_examples.py::test_layout_on_examples_with_derivatives[qmri_irt1-16] - AssertionError: assert 17 == 16
 +  where 17 = len([<BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_irt1/dataset_description.json'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_irt1/derivatives/qMRLab/dataset_description.json'>, <BIDSFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_irt1/derivatives/qMRLab/README'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_irt1/derivatives/qMRLab/sub-01/anat/sub-01_M0map.json'>, <BIDSImageFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_irt1/derivatives/qMRLab/sub-01/anat/sub-01_M0map.nii.gz'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_irt1/derivatives/qMRLab/sub-01/anat/sub-01_T1map.json'>, ...])
FAILED bids/layout/tests/test_layout_on_examples.py::test_layout_on_examples_with_derivatives[qmri_mp2rage-15] - AssertionError: assert 16 == 15
 +  where 16 = len([<BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_mp2rage/dataset_description.json'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_mp2rage/derivatives/pymp2rage/dataset_description.json'>, <BIDSFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_mp2rage/derivatives/pymp2rage/README'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_mp2rage/derivatives/pymp2rage/sub-1/anat/sub-1_T1map.json'>, <BIDSImageFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_mp2rage/derivatives/pymp2rage/sub-1/anat/sub-1_T1map.nii'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_mp2rage/derivatives/pymp2rage/sub-1/anat/sub-1_UNIT1.json'>, ...])
FAILED bids/layout/tests/test_layout_on_examples.py::test_layout_on_examples_with_derivatives[qmri_mtsat-24] - AssertionError: assert 26 == 24
 +  where 26 = len([<BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_mtsat/dataset_description.json'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_mtsat/derivatives/qMRLab/dataset_description.json'>, <BIDSFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_mtsat/derivatives/qMRLab/README'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_mtsat/derivatives/qMRLab/sub-01/anat/sub-01_M0map.json'>, <BIDSImageFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_mtsat/derivatives/qMRLab/sub-01/anat/sub-01_M0map.nii.gz'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_mtsat/derivatives/qMRLab/sub-01/anat/sub-01_MTRmap.json'>, ...])
FAILED bids/layout/tests/test_layout_on_examples.py::test_layout_on_examples_with_derivatives[qmri_vfa-18] - AssertionError: assert 19 == 18
 +  where 19 = len([<BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_vfa/dataset_description.json'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_vfa/derivatives/qMRLab/dataset_description.json'>, <BIDSFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_vfa/derivatives/qMRLab/README'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_vfa/derivatives/qMRLab/sub-01/anat/sub-01_M0map.json'>, <BIDSImageFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_vfa/derivatives/qMRLab/sub-01/anat/sub-01_M0map.nii.gz'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_vfa/derivatives/qMRLab/sub-01/anat/sub-01_T1map.json'>, ...])
FAILED bids/layout/tests/test_layout_on_examples.py::test_layout_on_examples_no_derivatives[qmri_megre-18] - AssertionError: assert 19 == 18
 +  where 19 = len([<BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_megre/dataset_description.json'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_megre/MEGRE.json'>, <BIDSFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_megre/README'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_megre/sub-01/anat/sub-01_echo-01_MEGRE.json'>, <BIDSImageFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_megre/sub-01/anat/sub-01_echo-01_MEGRE.nii.gz'>, <BIDSJSONFile filename='/home/runner/work/pybids/pybids/bids-examples/qmri_megre/sub-01/anat/sub-01_echo-02_MEGRE.json'>, ...])
FAILED bids/layout/tests/test_models.py::test_load_existing_config - sqlalchemy.exc.IntegrityError: (sqlite3.IntegrityError) UNIQUE constraint failed: configs.name
[SQL: INSERT INTO configs (name, _default_path_patterns) VALUES (?, ?)]
[parameters: ('dummy', 'null')]
(Background on this error at: https://sqlalche.me/e/14/gkpj)
FAILED bids/layout/tests/test_writing.py::TestWritableFile::test_build_path - sqlalchemy.orm.exc.DetachedInstanceError: Parent instance <BIDSFile at 0x7f842fd27c10> is not bound to a Session; lazy load operation of attribute 'tags' cannot proceed (Background on this error at: https://sqlalche.me/e/14/bhk3)
FAILED bids/layout/tests/test_writing.py::TestWritableFile::test_build_file - sqlalchemy.orm.exc.DetachedInstanceError: Instance <BIDSFile at 0x7f843038d190> is not bound to a Session; attribute refresh operation cannot proceed (Background on this error at: https://sqlalche.me/e/14/bhk3)
= 8 failed, 393 passed, 2 skipped, 2 xfailed, 131 warnings in 318.30s (0:05:18) =

so indeed would need into looking , probably starting from

FAILED bids/layout/tests/test_models.py::test_load_existing_config - sqlalchemy.exc.IntegrityError: (sqlite3.IntegrityError) UNIQUE constraint failed: configs.name
[SQL: INSERT INTO configs (name, _default_path_patterns) VALUES (?, ?)]
[parameters: ('dummy', 'null')]
(Background on this error at: https://sqlalche.me/e/14/gkpj)

@candleindark do you have any immediate ideas/past experience related to this?

@candleindark
Copy link

@candleindark do you have any immediate ideas/past experience related to this?

Sorry, I don't have any immediate experience with this error.

Some background information for you if you are not already aware about. SQLAlchemy 1.4 is a "transitional" version. You may want to use SQLAlchemy 2.0 style of working even in SQLAlchemy 1.4. For more information, please checkout "About this document" in https://docs.sqlalchemy.org/en/14/tutorial/index.html#sqlalchemy-1-4-2-0-tutorial.

@effigies
Copy link
Collaborator

Our goal is to drop sqlalchemy altogether. It will take some time.

@candleindark
Copy link

@candleindark do you have any immediate ideas/past experience related to this?

This page may be useful for you. For that particular error, you may want to look into https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#the-new-instance-conflicts-with-existing-identity-error-is-now-a-warning

@candleindark
Copy link

Our goal is to drop sqlalchemy altogether. It will take some time.

What are you going to replace it with? I always want to try https://www.edgedb.com/.

@effigies
Copy link
Collaborator

@DimitriPapadopoulos
Copy link
Contributor

Some dependencies have started dropping support for SQLAlchemy 1.3, not necessarily on purpose:
#981 (comment)

I bet this will keep popping up in the near future.

@effigies
Copy link
Collaborator

Replaced by #985.

@effigies effigies closed this Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants