Support for input from PENNPET Explorer#1028
Conversation
KrisThielemans
left a comment
There was a problem hiding this comment.
house-keeping comments. I didn't really check the actual code. Some general comments
- license info is sometimes LGPL
- copyright/authorship header is sometimes out-of-date
- we should never call
std::exitin library functions, as that will crash python etc. Callerror()instead
Co-authored-by: Kris Thielemans <[email protected]>
Co-authored-by: Kris Thielemans <[email protected]>
Co-authored-by: Kris Thielemans <[email protected]>
Co-authored-by: Kris Thielemans <[email protected]>
Co-authored-by: Kris Thielemans <[email protected]>
Co-authored-by: Kris Thielemans <[email protected]>
*. Fixed: - License is somethings LGPL - copyright / authorship - use error() instead of std::exit() - Some variables were shadowed. - Added doxygen comments. - Clean up
|
could you "resolve" comments when addressed? Makes it easier for me to find what I still need to look at. sorry |
|
I applied your comments. |
|
thanks. could you take a look at why the CI tests fail? |
|
WARNING: Scanner UPENN_5rings: inconsistent transaxial block info |
|
The |
* The utility can also decompress on-the-fly, introduce gaps
|
I see you've introduced a new "module" concept. I have no idea what that is supposed to be unfortunately and am therefore reluctant. (I see that by using Also, I don't see what is different between this scanner and others? I appreciate that you probably want to have gaps between buckets and not blocks, but maybe you can fake this at the moment by saying that one STIR block is the actual physical bucket? |
KrisThielemans
left a comment
There was a problem hiding this comment.
some more house-keeping but nearly done, except for Scanner mods, which I don't understand yet.
Note that probably all doxygen ingroup statements should be listmode, not IO``
Could you try to group updates in as few commits as possible. Also, ideally only push when done (to avoid multiple CI runs).
Co-authored-by: Kris Thielemans <[email protected]>
Co-authored-by: Kris Thielemans <[email protected]>
Co-authored-by: Kris Thielemans <[email protected]>
Co-authored-by: Kris Thielemans <[email protected]>
Co-authored-by: Kris Thielemans <[email protected]>
This reverts commit 1e7bbbd.
This reverts commit 17993b7.
This reverts commit 23ef00b.
* Reverted the Scanner mods (I don't really like the current scanner descriptions) * Pass test_Scanner * Updated and faster conv_UPENN_projdata_to_STIR * fixed in IO
KrisThielemans
left a comment
There was a problem hiding this comment.
it's looking good!
There's 2-3 unresolved very comments left (clock on the "conversations" in "Files changed").
squash-merge this ok?
|
I don't see any other comments. |
Successful unlisting of UPENN acquired data
Support for the 5-ring variant (with gaps)