Changed a lot after using pylint

[ShuchaoPangUNSW]

Hi Daniel and Phil,

I used the black and the pylint to check my scripts. And changed a lot now.

But I have to say that there are still several problems in formats reported from pylint below:

Besides, I also tested my parts using some testing cases to generate pet nifti files, starting from ./preprocess/data.py.

One more question is that as you see, I have used several default values or settings in ./pydicer/convert/convert_pt.py in order to get pet nifti files. As you know, some patients have some key property missings. So if we want to get their pet nifti data, we have to provide some default values or settings based on my script version. As you asked, why did I use this or that value? It is based on my conclusion from other patients with that key properties.

To be honest, I'm not an expert in converting pet dicom files into pet nifti files. But I had to do that for my projects, so I concluded more knowledge I learned online and some experience from others. And finally I completed this script to generate pet nifti files.

Welcome to receive your comments or change and test it.

Yours sincerely, Shuchao

[dalmouiee]

I might also add that if you could please use more informative names for the pull requests, and link it to a specific issue at the top, if there exists one it is solving :)

[pchlap]

One more question is that as you see, I have used several default values or settings in ./pydicer/convert/convert_pt.py in order to get pet nifti files. As you know, some patients have some key property missings. So if we want to get their pet nifti data, we have to provide some default values or settings based on my script version. As you asked, why did I use this or that value? It is based on my conclusion from other patients with that key properties.

To be honest, I'm not an expert in converting pet dicom files into pet nifti files. But I had to do that for my projects, so I concluded more knowledge I learned online and some experience from others. And finally I completed this script to generate pet nifti files.

I wonder if @rnfinnegan or @LoisHolloway have any suggestions on how to handle missing attributes when converting PET data? Is it safe to select default values and still get reasonable output? Or should we be erroring out on those (send to quarantine) for now?

We can discuss on Friday but I thought I would preemptively tag you both here in case you wanted to have a sneak peek before the meeting :)

[ShuchaoPangUNSW]

This is fantastic @ShuchaoPangUNSW! Great stuff on integrating this into the structure of the pydicer repository.

As @dalmouiee suggests, you can put (too-many-arguments, too-many-branches, too-many-locals, too-many-statements) into .pylintrc to disable them. I noticed there were a couple of suggestions from pylint to use f-strings everywhere possible rather than .format(). I do agree with pylint there so if you could resolve those in your code that would be great.

As you can see we have a few further suggestions, but these are now largely cosmetic and just cleaning up the code a bit. We can discuss the issue of choosing default values and how we want to handle that on Friday.

Thanks again Shuchao!

Hi Phil,

Can you further explain "to use f-strings everywhere possible rather than .format()."? I also want to reduce this kind of comments from pylint, but no experience on this. Up to now, three comments are left below:

[pchlap]

Hi @ShuchaoPangUNSW,

Can you further explain "to use f-strings everywhere possible rather than .format()."? I also want to reduce this kind of comments from pylint, but no experience on this. Up to now, three comments are left below:

Yep so since Python version 3.6 a feature called f-strings was introduced to make formatting strings (inserting variables into strings etc) easier. They are now the preferred way to format strings in Python, since they are easier to read, more stable and faster. The old way of using .format() still works for compatibility with old code, but ideally we will all now use f-strings in new projects.

So, instead of doing something like:

"my string with number: {0}".format(my_number)

you should now always use an f-string like:

f"my string with number: {my_number}"

You can read more info on f-strings in Python here: https://realpython.com/python-f-strings/

Don't worry about the too-many- recommendations from pylint. They are very strict and we have disabled them in pull request #44. We will need to merge in those changes in to your branch, and then pylint will be happy here :)