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.

Sign up for GitHub

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

Jump to bottom

Beginning update reproin.py to deal with ses-X* instances #592

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Beginning update reproin.py to deal with ses-X* instances #592

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7be095
Update reproin.py
Michael-Sun Sep 23, 2022
4e54f26
Update reproin to deal with ses-(X|x)*
Michael-Sun Sep 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions heudiconv/heuristics/reproin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@

- We replace all ( with { and ) with } to be able e.g. to specify session {date}
- "WIP " prefix unconditionally added by the scanner is stripped


-Michael-Sun Plans on adding a heuristic to deal with ses-X* instances
Copy link
Member

Choose a reason for hiding this comment

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

make it happen!

Copy link
Member

Choose a reason for hiding this comment

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

add above , where _ses- marker described, description that XXX markers are dealt in some other way.

"""

import os
Expand Down Expand Up @@ -780,6 +783,13 @@ def infotoids(seqinfos, outdir):
"Should have got a single session marker. Got following: %s"
% ', '.join(map(repr, ses_markers))
)
if re.match("(X|x)*", ses_markers):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if re.match("(X|x)*", ses_markers):
if re.match("XX+$", ses_markers) or re.match("xx+$", ses_markers):

to

  • ensure consistent xxx or XXX and not XxX
  • require at least 2 of those xs
  • no other symbol there (anchor)

# replace X* or x* sessions with prior_sessions+1
prior_sessions = sorted(glob(os.path.join(sessions_dir, 'ses-*')))
if prior_sessions:
session = '%03d' % (len(prior_sessions) + 1)
else:
session='001'
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should hardcode to %03d since intention on formatting might have been different etc.

  • do not just take the length of prior_sessions but
    • extract labels (what comes after ses-), see what length they have. if non-uniform (1,...,12) - use plain '%d'. If uniform - use f'%0{len(ses_labels[0])}d'`
    • do not base on len(prior_sessions) but convert all session labels into int. If errors - errors. Take max of those ints, so smth like max(map(int, ses_labels))
  • make this logic into a function get_next_xxx_session(prior_session_dirs: list[str]), for which also add unittests for it

session = ses_markers[0]
else:
# TODO - I think we are doomed to go through the sequence and split
Expand Down