Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pull 26.0.1 and 26.0.2 bug fixes into main #1233

Merged
merged 7 commits into from
Mar 17, 2025

Conversation

laemtl
Copy link
Contributor

@laemtl laemtl commented Feb 24, 2025

Replace #1225

@laemtl laemtl requested a review from cmadjar February 24, 2025 08:23
@maximemulder
Copy link
Contributor

Regarding the CI failures:

  • The "XXX is deprecated" type checks are not blocking, you can ignore them if you want to.
  • The blocking type check is Error: Import "delete_physiological_file" could not be resolved. You can either use from scripts.delete_physiological_file import xxx or (preferably) put the function you need somewhere in the lib modules.
  • You can use ruff check --fix to sort imports and fix trivial formatting issues.

Copy link
Contributor

@maximemulder maximemulder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the PR ! Small things I found while reading the code.

@maximemulder maximemulder dismissed their stale review February 24, 2025 11:13

Halp I got it wrong.

Copy link
Contributor

@maximemulder maximemulder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review take two.

@laemtl
Copy link
Contributor Author

laemtl commented Feb 24, 2025

Will test and fix the static errors soon! Blocked.

@laemtl laemtl added the Blocked Merge it and you die label Feb 24, 2025
@cmadjar cmadjar added this to the 27.0 milestone Feb 26, 2025
@laemtl laemtl removed the Blocked Merge it and you die label Mar 17, 2025
@laemtl laemtl force-pushed the 26.0.1-and-26.0.2-bugfixes branch from c9e4d19 to af59df9 Compare March 17, 2025 05:02
@laemtl laemtl requested a review from maximemulder March 17, 2025 05:10
@laemtl
Copy link
Contributor Author

laemtl commented Mar 17, 2025

  • The blocking type check is Error: Import "delete_physiological_file" could not be resolved. You can either use from scripts.delete_physiological_file import xxx or (preferably) put the function you need somewhere in the lib modules.

I left it in script because delete_physiological_file is a script, not a lib.

Copy link
Contributor

@maximemulder maximemulder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, I added a commit to set the new script as executable:

git update-index --chmod=+x python/scripts/delete_physiological_file.py

The new script works on my machine (although not sure if we really want to print the paths as {FilePath: '...'}) !

(loris-mri-python) lorisadmin@loris:/opt/loris/bin/mri$ delete_physiological_file.py --fileid 1 --profile database_config.py --confirm --deleteondisk

Archives
----------------------------
({'FilePath': 'bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT167/ses-V1/eeg/sub-OTT167_ses-V1_task-faceO_eeg.tgz'},)

Physiological File
----------------------------
({'FilePath': 'bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT167/ses-V1/eeg/sub-OTT167_ses-V1_task-faceO_eeg.edf'},)

Physiological Metadata File
----------------------------
()

Channels
----------------------------
({'FilePath': 'bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT167/ses-V1/eeg/sub-OTT167_ses-V1_task-faceO_channels.tsv'},)

Electrodes
----------------------------
({'FilePath': 'bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT167/ses-V1/eeg/sub-OTT167_ses-V1_task-faceO_electrodes.tsv'},)

Coordinate Systems
----------------------------
()

Event Files
----------------------------
({'FilePath': 'bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT167/ses-V1/eeg/sub-OTT167_ses-V1_task-faceO_events.tsv'},)

Event Archives
----------------------------
()

Chunks
----------------------------
({'FilePath': 'bids_imports/Face13_BIDSVersion_1.1.0_chunks/sub-OTT167_ses-V1_task-faceO_eeg.chunks'},)

Dropping all DB entries for physiological file: 1

----------------------------

Delete physiological_event_parameter_category_level

Delete physiological_event_parameter

Delete physiological_channel

Delete physiological_electrode

Delete physiological_coord_system_point_3d_rel

Delete point_3d

Delete physiological_coord_system

Delete physiological_coord_system_electrode_rel

Delete physiological_electrode

Delete physiological_parameter_file

Delete physiological_archive

Delete physiological_event_archive

Delete physiological_task_event_opt

Delete physiological_task_event_hed_rel

Delete physiological_task_event

Delete physiological_event_file

Delete physiological_file


Deleting files on disk for physiological file: 1

----------------------------

Deleting file bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT167/ses-V1/eeg/sub-OTT167_ses-V1_task-faceO_eeg.tgz

Deleting file bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT167/ses-V1/eeg/sub-OTT167_ses-V1_task-faceO_eeg.edf

Deleting file bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT167/ses-V1/eeg/sub-OTT167_ses-V1_task-faceO_channels.tsv

Deleting file bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT167/ses-V1/eeg/sub-OTT167_ses-V1_task-faceO_electrodes.tsv

Deleting file bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT167/ses-V1/eeg/sub-OTT167_ses-V1_task-faceO_events.tsv

@cmadjar cmadjar merged commit 36aa8e4 into aces:main Mar 17, 2025
9 checks passed
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.

3 participants