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

Place containers in yoda-idiomatic spot #121

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

asmacdo
Copy link
Member

@asmacdo asmacdo commented May 24, 2024

When I wrote this originally I did:
datalad install -d . ///repronim/containers code/containers

But I should have used -s:
datalad install -d . -s ///repronim/containers code/containers

@yarikoptic
Copy link
Member

where is syntax error? there seems to be full refactoring on where to place the containers subdataset... and it should be harmoneous with the way we do in the monolythic script we also have in README and then test on CI.

Please make description proper and ensure consistency

@asmacdo asmacdo changed the title Fix syntax error in README Place containers in yoda-idiomatic spot May 25, 2024
@asmacdo asmacdo force-pushed the fix-install-usage branch from 7ab05d9 to 106222b Compare May 25, 2024 20:39
@asmacdo
Copy link
Member Author

asmacdo commented May 25, 2024

the "syntax error" actually never made it into the README, my mistake.

@asmacdo asmacdo force-pushed the fix-install-usage branch from 0be752d to 3ef840b Compare May 30, 2024 21:16
.gitmodules Outdated Show resolved Hide resolved
README.md Outdated
--input sourcedata \
--output . \
'{inputs}' '{outputs}' participant group -w workdir
'{inputs}' '{outputs}' participant --participant-label sub-02 -w workdir
)
Copy link
Member

Choose a reason for hiding this comment

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

whatever you need to change, change in your step by step version, not here!

@yarikoptic
Copy link
Member

I believe that your changes to monolythic version lead to some odd CI fails starting to happen (I didn't analyze)

Fatal Python error: init_fs_encoding: failed to get the Python codec of the filesystem encoding

since daily testing performed just fine: https://github.com/ReproNim/containers/actions/runs/9376864122/job/25817510428

I can only repeat again -- do not change original monolythic version, change your step by step to match in terms of commands!

@asmacdo asmacdo marked this pull request as draft June 5, 2024 12:43
@asmacdo asmacdo force-pushed the fix-install-usage branch from 3ef840b to a27bb78 Compare June 5, 2024 15:38
@asmacdo
Copy link
Member Author

asmacdo commented Jun 5, 2024

This works on typhon and CI passes. Doesn't run for me locally, but that is tracked #125 and shouldnt block merge here.

@asmacdo asmacdo marked this pull request as ready for review June 5, 2024 15:52
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@asmacdo asmacdo marked this pull request as draft June 5, 2024 19:23
@asmacdo asmacdo force-pushed the fix-install-usage branch from a27bb78 to 27a5548 Compare June 5, 2024 19:23
@asmacdo asmacdo force-pushed the fix-install-usage branch from 27a5548 to d61b1e3 Compare June 5, 2024 19:43
@asmacdo asmacdo marked this pull request as ready for review June 5, 2024 19:44
README.md Outdated Show resolved Hide resolved
@yarikoptic yarikoptic merged commit 7f4bb48 into ReproNim:master Jun 11, 2024
5 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.

2 participants