search

AustralianCancerDataNetwork / pydicer

PYthon Dicom Image ConvertER
https://australiancancerdatanetwork.github.io/pydicer/
Apache License 2.0
20 stars 5 forks source link

First pipeline #28

Closed dalmouiee closed 3 years ago

dalmouiee commented 3 years ago

Pipeline for checking pushs to Main and pull requests into Main

dalmouiee commented 3 years ago

Interesting to see that the pipeline failed for python 3.6 and 3.9 but worked for 3.7 3.8. The fails were about cconflicting versions of SimpleITK that is defined for Platipy (when we import Platipy), and the SimpleITK version we define in the the requirements.txt.

pchlap commented 3 years ago

Yep that is interesting. I can make a change to platipy to clear up the issue for Python 3.9... The one for 3.6 might be a problem, because SimpleITK 2.0.0 requires 3.7 as a minimum. There was quite a few changes from SimpleITK 1 -> 2 so we need to talk about if we want to support both or not...

dalmouiee commented 3 years ago

We could make the assumption that python3.7+ is in use for development and then sort this out later on for all version of Python 3?

pchlap commented 3 years ago

We could make the assumption that python3.7+ is in use for development and then sort this out later on for all version of Python 3?

Alternatively, can you try changing the required version of SimpleITK to 2.0.2 in this repo? I think that might fix it...

dalmouiee commented 3 years ago

We could make the assumption that python3.7+ is in use for development and then sort this out later on for all version of Python 3?

Alternatively, can you try changing the required version of SimpleITK to 2.0.2 in this repo? I think that might fix it...

I changed this to 2.0.2, but now we get errors about pymedphys for python3.6 and platipy for python3.9, the other two are working fine

pchlap commented 3 years ago

We could make the assumption that python3.7+ is in use for development and then sort this out later on for all version of Python 3?

Alternatively, can you try changing the required version of SimpleITK to 2.0.2 in this repo? I think that might fix it...

I changed this to 2.0.2, but now we get errors about pymedphys for python3.6 and platipy for python3.9, the other two are working fine

Yeah pymedphys requires 3.7 as a minimum... I think we might need to support 3.7 as a minimum as well? At least for now?

pchlap commented 3 years ago

We could make the assumption that python3.7+ is in use for development and then sort this out later on for all version of Python 3?

Alternatively, can you try changing the required version of SimpleITK to 2.0.2 in this repo? I think that might fix it...

I changed this to 2.0.2, but now we get errors about pymedphys for python3.6 and platipy for python3.9, the other two are working fine

Yeah pymedphys requires 3.7 as a minimum... I think we might need to support 3.7 as a minimum as well? At least for now?

And to solve the issue in 3.9, try setting pydicom in pydicer to require at least 2.1.2... See if that helps. Not sure why this is only an error in Python 3.9 though...

dalmouiee commented 3 years ago

We could make the assumption that python3.7+ is in use for development and then sort this out later on for all version of Python 3?

Alternatively, can you try changing the required version of SimpleITK to 2.0.2 in this repo? I think that might fix it...

I changed this to 2.0.2, but now we get errors about pymedphys for python3.6 and platipy for python3.9, the other two are working fine

Yeah pymedphys requires 3.7 as a minimum... I think we might need to support 3.7 as a minimum as well? At least for now?

And to solve the issue in 3.9, try setting pydicom in pydicer to require at least 2.1.2... See if that helps. Not sure why this is only an error in Python 3.9 though...

Yeah the issue is still persisting for 3.9, seems that there is some discrepancy between platipy and pydicom. Is it safe to assume it works for 3.7 and 3.8 for development now and leave out 3.9? It might cause us issues later on when we want to solve this issue when the tool is more complex... I'm running 3.8.10 on my mac but not sure about you and the others for development

pchlap commented 3 years ago

We could make the assumption that python3.7+ is in use for development and then sort this out later on for all version of Python 3?

Alternatively, can you try changing the required version of SimpleITK to 2.0.2 in this repo? I think that might fix it...

I changed this to 2.0.2, but now we get errors about pymedphys for python3.6 and platipy for python3.9, the other two are working fine

Yeah pymedphys requires 3.7 as a minimum... I think we might need to support 3.7 as a minimum as well? At least for now?

And to solve the issue in 3.9, try setting pydicom in pydicer to require at least 2.1.2... See if that helps. Not sure why this is only an error in Python 3.9 though...

Yeah the issue is still persisting for 3.9, seems that there is some discrepancy between platipy and pydicom. Is it safe to assume it works for 3.7 and 3.8 for development now and leave out 3.9? It might cause us issues later on when we want to solve this issue when the tool is more complex... I'm running 3.8.10 on my mac but not sure about you and the others for development

Yeah I'd say lets disable 3.9 from the pipeline for now. I'll add an issue to investigate whats going on with platipy and we'll make sure we resolve that for a future version.

dalmouiee commented 3 years ago

@pchlap Removed 3.6/3.9 from pipeline, if all looks good, happy to confirm and merge :)