inm7/inm-icf-utilities
10
0
Fork 0
Code Issues 6 Pull requests 5 Packages Activity Actions

Code changes to make this repo compatible with datalad-catalog>=1.1.0 #46

Open
jsheunis wants to merge 1 commit from jsheunis/catalog-update-fix into main
pull from: jsheunis/catalog-update-fix
merge into: inm7:main
Conversation 5 Commits 1 Files changed 2 +51 -83
jsheunis commented 2024-01-10 08:24:09 +00:00 (Migrated from github.com)
Copy link

Closes https://github.com/psychoinformatics-de/inm-icf-utilities/issues/43

Closes https://github.com/psychoinformatics-de/inm-icf-utilities/issues/43
jsheunis commented 2024-01-10 13:23:34 +00:00 (Migrated from github.com)
Copy link

The appveyor run fails because it still installs the older version of datalad-catalog via the singularity image: https://ci.appveyor.com/project/mih/inm-icf-utilities/builds/48919129#L475. The PR branch is not checked out by this code, therefore it will always take the requirements-devel.txt file state from the main branch of the remote at https://github.com/psychoinformatics-de/inm-icf-utilities, which means testing for dependency updates cannot occur usefully via this standard PR testing route.

Same problem noted here: https://github.com/psychoinformatics-de/inm-icf-utilities/pull/42#issuecomment-1847161161

The appveyor run fails because it still installs the older version of `datalad-catalog` via the singularity image: https://ci.appveyor.com/project/mih/inm-icf-utilities/builds/48919129#L475. The PR branch is not checked out by this code, therefore it will always take the `requirements-devel.txt` file state from the `main` branch of the remote at https://github.com/psychoinformatics-de/inm-icf-utilities, which means testing for dependency updates cannot occur usefully via this standard PR testing route. Same problem noted here: https://github.com/psychoinformatics-de/inm-icf-utilities/pull/42#issuecomment-1847161161
mslw (Migrated from github.com) approved these changes 2024-09-12 13:02:04 +00:00
mslw (Migrated from github.com) left a comment
Copy link

Bundling an updated catalog would have several advantages, incl. fixing the caching issues (datalad/datalad-catalog#457) recently reported by e-mail by @ste-phi

@jsheunis it's been a while but do you think it's good to go?

It builds a catalog for me when tested on some of my local data, so for me that's a yes.

Bundling an updated catalog would have several advantages, incl. fixing the caching issues (datalad/datalad-catalog#457) recently reported by e-mail by @ste-phi @jsheunis it's been a while but do you think it's good to go? It builds a catalog for me when tested on some of my local data, so for me that's a yes.
bin/catalogify_studyvisit_from_meta
@ -123,3 +134,4 @@
)
return meta_item
mslw (Migrated from github.com) commented 2024-09-12 12:54:15 +00:00
Copy link

Maybe - just maybe - this could have default tab set to subdatasets (the study dataset has only subdatasets, no content) but IIRC catalog_add can take only a config file, not an individual option, so I'm not inclined to change it here.

Maybe - just maybe - this could have default tab set to subdatasets (the study dataset has only subdatasets, no content) but IIRC `catalog_add` can take only a config file, not an individual option, so I'm not inclined to change it here.
requirements-devel.txt
@ -5,3 +5,3 @@
pytest-env
datalad-catalog==0.2.1b0 --pre
datalad-catalog
www-authenticate
mslw (Migrated from github.com) commented 2024-09-12 12:56:38 +00:00
Copy link

I wonder if it would make sense to still pin the version, just to the current one, to avoid having to deal with future changes? Or is that unnecessary?

datalad-catalog==1.1.1
I wonder if it would make sense to still pin the version, just to the current one, to avoid having to deal with future changes? Or is that unnecessary? ```suggestion datalad-catalog==1.1.1 ```
jsheunis (Migrated from github.com) reviewed 2024-09-13 07:48:01 +00:00
bin/catalogify_studyvisit_from_meta
@ -123,3 +134,4 @@
)
return meta_item
jsheunis (Migrated from github.com) commented 2024-09-13 07:48:01 +00:00
Copy link

but IIRC catalog_add can take only a config file, not an individual option

That's correct. There was an intention to update the code of catalog_set to allow the config option, which would reset the config. But this hasn't been implemented.

> but IIRC catalog_add can take only a config file, not an individual option That's correct. There was an intention to update the code of `catalog_set` to allow the `config` option, which would reset the config. But this hasn't been implemented.
jsheunis (Migrated from github.com) reviewed 2024-09-13 07:49:05 +00:00
requirements-devel.txt
@ -5,3 +5,3 @@
pytest-env
datalad-catalog==0.2.1b0 --pre
datalad-catalog
www-authenticate
jsheunis (Migrated from github.com) commented 2024-09-13 07:49:05 +00:00
Copy link

I think ideally we wouldn't pin it, but under the circumstances of datalad-catalog receiving breaking changes sometimes, and the future development being unclear at the moment, I would agree with pinning it here.

I think ideally we wouldn't pin it, but under the circumstances of `datalad-catalog` receiving breaking changes sometimes, and the future development being unclear at the moment, I would agree with pinning it here.
jsheunis commented 2024-09-13 08:27:34 +00:00 (Migrated from github.com)
Copy link

Looks good. I reran the PR appveyor job and all are successful now.

Looks good. I reran the PR appveyor job and all are successful now.
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin jsheunis/catalog-update-fix:jsheunis/catalog-update-fix
git switch jsheunis/catalog-update-fix

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff jsheunis/catalog-update-fix
git switch jsheunis/catalog-update-fix
git rebase main
git switch main
git merge --ff-only jsheunis/catalog-update-fix
git switch jsheunis/catalog-update-fix
git rebase main
git switch main
git merge --no-ff jsheunis/catalog-update-fix
git switch main
git merge --squash jsheunis/catalog-update-fix
git switch main
git merge --ff-only jsheunis/catalog-update-fix
git switch main
git merge jsheunis/catalog-update-fix
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
mslw
Labels
Clear labels   
bug
Something isn't working

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
wontfix
This will not be worked on

No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
inm7/inm-icf-utilities!46
No description provided.