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

script to remove dups from intendedfor #28

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

spacetop-admin
Copy link
Contributor

No description provided.


dup_fname = os.path.basename(dup_fpath)
sub = [match for match in dup_fname.split('_') if "sub" in match][0]
ses = [match for match in dup_fname.split('_') if "ses" in match][0]
Copy link
Collaborator

@yarikoptic yarikoptic Mar 23, 2022

Choose a reason for hiding this comment

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

  1. if I were to parse the filename into a set of entities, I would have done it into a dict, like the following:
    smth like this:
entities = dict(
   match.split('-')
   for match in dup_fname.split('_')
   if '-' in match
)
  1. ideally don't bother coding yourself. There is now in heudiconv a BIDSFile helper: have a look at https://github.com/nipy/heudiconv/blob/e747f0595dd393384d65219f5137d42b2a5c8e73/heudiconv/bids.py#L938 and its usage, in particular in tests

Copy link
Collaborator

Choose a reason for hiding this comment

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

installation heudiconv from conda is via conda install -f conda-forge heudiconv which should have now 0.10.0 release

@yarikoptic
Copy link
Collaborator

@jungheejung Ideally you should have switched to the main branch (master) for unrelated to this PR (script to remove dups...) changes, and commit there so this PR would correspond only to the single feature related to this PR.

@spacetop-admin
Copy link
Contributor Author

spacetop-admin commented Mar 24, 2022 via email

@yarikoptic
Copy link
Collaborator

yarikoptic commented Mar 24, 2022

not really "Revert" -- let's do tomorrow in CLInic (add to the agenda plz)? and meanwhile can continue working in this branch, just make sure having separate commits for separate "pieces" -- it will be easy to split it all apart later on.

@spacetop-admin
Copy link
Contributor Author

spacetop-admin commented Mar 24, 2022 via email

jungheejung and others added 3 commits March 25, 2022 11:22
instead of matching, simply call json key from json object

Co-authored-by: Yaroslav Halchenko <debian@onerussian.com>
@spacetop-admin
Copy link
Contributor Author

@all-contributors please add @yarikoptic for infrastructure, tests and code

@allcontributors
Copy link

@spacetop-admin

I've put up a pull request to add @yarikoptic! 🎉

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.

None yet

3 participants