nipy / heudiconv

Flexible DICOM conversion into structured directory layouts
https://heudiconv.readthedocs.io
Other
234 stars 125 forks source link

Check if acq_time has ms and add .0 if not #680

Closed crcox closed 1 year ago

crcox commented 1 year ago
yarikoptic commented 1 year ago

we already do that since today's release 0.13.1 at https://github.com/nipy/heudiconv/blob/719ad9d4688e625643d01766653789abdf7bc5ca/heudiconv/utils.py#L669:5

did you find it not working ?

yarikoptic commented 1 year ago

sorry, I misread your report -- you are already stating that our helper doesn't do what it supposed to ... I will check later in detail.

yarikoptic commented 1 year ago
  • In python 3.11.2, strptime_micr(acq_date + acq_time, "%Y%m%d%H%M%S[.%f]") does not make ms optional

why exactly you think so, i.e. do you have an example to demonstrate that?

from me reading the code here we add %f only if provided date has ms and otherwise uses the pattern to match without %f so I do think this patch is needed. FWIW here is the test which tests this code

crcox commented 1 year ago

Two apologies: I must not have tested appropriately--your test proves that it works. Maybe I was not using the version of Python that I thought I was.

I can say for certain that when running the main script, the code would crash when calling this function; manually revising the fmt string resolved the problem, as did my solution above. So really, the only thing that makes sense is I was not running the version of Python I thought I was.

Second apology is for the slow reply.

yarikoptic commented 1 year ago

so some mystery remains -- thus please reopen or file a new one if you somehow see this issue to come up.