PythonPlot -> CI failing

[aTrotier]

Hi @mfuderer the CI is failing because PythonPlot is not a dependencies of the package (and should not be)

If you really want to add this possibility maybe we should use the extension possibility. See -> https://pkgdocs.julialang.org/v1/creating-packages/#Conditional-loading-of-code-in-packages-(Extensions)

Can you make a PR request rather than working directly on the main branch ? We can check if the CI passes before merging.

[mfuderer]

Hi Aurelien, Sorry for goofing around. Not being too experienced with the usage of git, I pushed it (well, actually pull-requested it) without immediately relaizing what was happening. On top, I did not manage to run a test, since QMRIColors somehow kept refering to the online package version. Lastly, I accept your argument that PythonPlot should not be a dependency of the package. This immediately nulls the main reason for my change. So, I suggest that you reject whichever incoming commits I made. I will then make a new branch with my changes, but backtrack all these that would require PythonPlot (i.e., keep only some minor repairs to the .md files). Then I'll publish the new branch and you can have a look whether it is worthwhile merging it. Best, Miha

…

________________________________ From: aTrotier ***@***.***> Sent: Friday, January 24, 2025 15:57 To: MagneticResonanceImaging/QMRIColors.jl ***@***.***> Cc: Fuderer, M. (Miha) ***@***.***>; Mention ***@***.***> Subject: [MagneticResonanceImaging/QMRIColors.jl] PythonPlot -> CI failing (Issue #8) Hi @mfuderer<https://github.com/mfuderer> the CI is failing because PythonPlot is not a dependencies of the package (and should not be) If you really want to add this possibility maybe we should use the extension possibility. See -> https://pkgdocs.julialang.org/v1/creating-packages/#Conditional-loading-of-code-in-packages-(Extensions) Can you make a PR request rather than working directly on the main branch ? We can check if the CI passes before merging. — Reply to this email directly, view it on GitHub<#8>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BB2W4ZHSYOAURGIDHMQRS5D2MJIDZAVCNFSM6AAAAABVZ3OJOSVHI2DSMVQWIX3LMV43ASLTON2WKOZSHAYDSNRVHA3TOMQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

------------------------------------------------------------------------------ De informatie opgenomen in dit bericht kan vertrouwelijk zijn en is uitsluitend bestemd voor de geadresseerde. Indien u dit bericht onterecht ontvangt, wordt u verzocht de inhoud niet te gebruiken en de afzender direct te informeren door het bericht te retourneren. Het Universitair Medisch Centrum Utrecht is een publiekrechtelijke rechtspersoon in de zin van de W.H.W. (Wet Hoger Onderwijs en Wetenschappelijk Onderzoek) en staat geregistreerd bij de Kamer van Koophandel voor Midden-Nederland onder nr. 30244197. Denk s.v.p aan het milieu voor u deze e-mail afdrukt.

------------------------------------------------------------------------------ This message may contain confidential information and is intended exclusively for the addressee. If you receive this message unintentionally, please do not use the contents but notify the sender immediately by return e-mail. University Medical Center Utrecht is a legal person by public law and is registered at the Chamber of Commerce for Midden-Nederland under no. 30244197. Please consider the environment before printing this e-mail.

[aTrotier]

On top, I did not manage to run a test, since QMRIColors somehow kept refering to the online package version.

To run the test locally you can clone the repository -> activate the julia environment of QMRIColors and (in the pkg mode) type `test``
For the documentation, it is a little bit different.

Lastly, I accept your argument that PythonPlot should not be a dependency of the package. This immediately nulls the main reason for my change.

I don't use PythonPlot, does the function : PythonPlot.ColorMap returns a python object or just a specific array / matrix format that we can easily build with standard julia function without relaying on this package ?

So, I suggest that you reject whichever incoming commits I made. I will then make a new branch with my changes, but backtrack all these that would require PythonPlot (i.e., keep only some minor repairs to the .md files). Then I'll publish the new branch and you can have a look whether it is worthwhile merging it.

The main branch is now protected from direct push (just through pull-request). We will still be able to create branch on the repository without having a fork it.

@mfuderer can you try to create a branch, delete the PythonPlot requirements and make a pull-request, we will see if the CI is working :)

[mfuderer]

Sure. Will follow up tomorrow

…

________________________________ From: aTrotier ***@***.***> Sent: Tuesday, January 28, 2025 12:47 To: MagneticResonanceImaging/QMRIColors.jl ***@***.***> Cc: Fuderer, M. (Miha) ***@***.***>; Mention ***@***.***> Subject: Re: [MagneticResonanceImaging/QMRIColors.jl] PythonPlot -> CI failing (Issue #8) On top, I did not manage to run a test, since QMRIColors somehow kept refering to the online package version. To run the test locally you can clone the repository -> activate the julia environment of QMRIColors and (in the pkg mode) type `test`` For the documentation, it is a little bit different. Lastly, I accept your argument that PythonPlot should not be a dependency of the package. This immediately nulls the main reason for my change. I don't use PythonPlot, does the function : PythonPlot.ColorMap returns a python object or just a specific array / matrix format that we can easily build with standard julia function without relaying on this package ? So, I suggest that you reject whichever incoming commits I made. I will then make a new branch with my changes, but backtrack all these that would require PythonPlot (i.e., keep only some minor repairs to the .md files). Then I'll publish the new branch and you can have a look whether it is worthwhile merging it. The main branch is now protected from direct push (just through pull-request). We will still be able to create branch on the repository without having a fork it. @mfuderer<https://github.com/mfuderer> can you try to create a branch, delete the PythonPlot requirements and make a pull-request, we will see if the CI is working :) — Reply to this email directly, view it on GitHub<#8 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BB2W4ZB6DNDH42BSHRF5UDL2M5U3ZAVCNFSM6AAAAABVZ3OJOSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJYG43DGNRZGE>. You are receiving this because you were mentioned.Message ID: ***@***.***>

------------------------------------------------------------------------------ De informatie opgenomen in dit bericht kan vertrouwelijk zijn en is uitsluitend bestemd voor de geadresseerde. Indien u dit bericht onterecht ontvangt, wordt u verzocht de inhoud niet te gebruiken en de afzender direct te informeren door het bericht te retourneren. Het Universitair Medisch Centrum Utrecht is een publiekrechtelijke rechtspersoon in de zin van de W.H.W. (Wet Hoger Onderwijs en Wetenschappelijk Onderzoek) en staat geregistreerd bij de Kamer van Koophandel voor Midden-Nederland onder nr. 30244197. Denk s.v.p aan het milieu voor u deze e-mail afdrukt.

------------------------------------------------------------------------------ This message may contain confidential information and is intended exclusively for the addressee. If you receive this message unintentionally, please do not use the contents but notify the sender immediately by return e-mail. University Medical Center Utrecht is a legal person by public law and is registered at the Chamber of Commerce for Midden-Nederland under no. 30244197. Please consider the environment before printing this e-mail.

[mfuderer]

Re-examining my changes, the ONLY changes I made relate to the conversion (via PythonPlot.ColorMap) of an rgb-array into a Python-specific object. With that, and with the choice not to make PythonPlot a dependency, this removes the relevance of all my changes. With that, I assume that no further action is needed

…

________________________________ From: aTrotier ***@***.***> Sent: Tuesday, January 28, 2025 12:47 To: MagneticResonanceImaging/QMRIColors.jl ***@***.***> Cc: Fuderer, M. (Miha) ***@***.***>; Mention ***@***.***> Subject: Re: [MagneticResonanceImaging/QMRIColors.jl] PythonPlot -> CI failing (Issue #8) On top, I did not manage to run a test, since QMRIColors somehow kept refering to the online package version. To run the test locally you can clone the repository -> activate the julia environment of QMRIColors and (in the pkg mode) type `test`` For the documentation, it is a little bit different. Lastly, I accept your argument that PythonPlot should not be a dependency of the package. This immediately nulls the main reason for my change. I don't use PythonPlot, does the function : PythonPlot.ColorMap returns a python object or just a specific array / matrix format that we can easily build with standard julia function without relaying on this package ? So, I suggest that you reject whichever incoming commits I made. I will then make a new branch with my changes, but backtrack all these that would require PythonPlot (i.e., keep only some minor repairs to the .md files). Then I'll publish the new branch and you can have a look whether it is worthwhile merging it. The main branch is now protected from direct push (just through pull-request). We will still be able to create branch on the repository without having a fork it. @mfuderer<https://github.com/mfuderer> can you try to create a branch, delete the PythonPlot requirements and make a pull-request, we will see if the CI is working :) — Reply to this email directly, view it on GitHub<#8 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BB2W4ZB6DNDH42BSHRF5UDL2M5U3ZAVCNFSM6AAAAABVZ3OJOSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJYG43DGNRZGE>. You are receiving this because you were mentioned.

------------------------------------------------------------------------------ De informatie opgenomen in dit bericht kan vertrouwelijk zijn en is uitsluitend bestemd voor de geadresseerde. Indien u dit bericht onterecht ontvangt, wordt u verzocht de inhoud niet te gebruiken en de afzender direct te informeren door het bericht te retourneren. Het Universitair Medisch Centrum Utrecht is een publiekrechtelijke rechtspersoon in de zin van de W.H.W. (Wet Hoger Onderwijs en Wetenschappelijk Onderzoek) en staat geregistreerd bij de Kamer van Koophandel voor Midden-Nederland onder nr. 30244197. Denk s.v.p aan het milieu voor u deze e-mail afdrukt.

------------------------------------------------------------------------------ This message may contain confidential information and is intended exclusively for the addressee. If you receive this message unintentionally, please do not use the contents but notify the sender immediately by return e-mail. University Medical Center Utrecht is a legal person by public law and is registered at the Chamber of Commerce for Midden-Nederland under no. 30244197. Please consider the environment before printing this e-mail.

[aTrotier]

Oh too bad.

Like I said, we have 2 options :

1. Because it only takes a line of code I don't think it is an issue for the user to convert it by themself especially if we add a mention in the documentation.
2. We add an [extension] PythonPlot that is compiled only if the user add it.

In my opinion, it will be easier to not have to maintain a lot of plot packages.

In that case should we revert to the version before your commit ? (even in the readme...)

[mfuderer]

Agree. So we stay on option 1.

…

________________________________ From: aTrotier ***@***.***> Sent: Wednesday, January 29, 2025 11:45 To: MagneticResonanceImaging/QMRIColors.jl ***@***.***> Cc: Fuderer, M. (Miha) ***@***.***>; Mention ***@***.***> Subject: Re: [MagneticResonanceImaging/QMRIColors.jl] PythonPlot -> CI failing (Issue #8) Oh too bad. Like I said, we have 2 options : 1. Because it only takes a line of code I don't think it is an issue for the user to convert it by themself especially if we add a mention in the documentation. 2. We add an [extension] PythonPlot that is compiled only if the user add it. In my opinion, it will be easier to not have to maintain a lot of plot packages. — Reply to this email directly, view it on GitHub<#8 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BB2W4ZCDAHNBFH3FID3MSTL2NCWMNAVCNFSM6AAAAABVZ3OJOSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRRGI4DIOBZG4>. You are receiving this because you were mentioned.Message ID: ***@***.***>

------------------------------------------------------------------------------ De informatie opgenomen in dit bericht kan vertrouwelijk zijn en is uitsluitend bestemd voor de geadresseerde. Indien u dit bericht onterecht ontvangt, wordt u verzocht de inhoud niet te gebruiken en de afzender direct te informeren door het bericht te retourneren. Het Universitair Medisch Centrum Utrecht is een publiekrechtelijke rechtspersoon in de zin van de W.H.W. (Wet Hoger Onderwijs en Wetenschappelijk Onderzoek) en staat geregistreerd bij de Kamer van Koophandel voor Midden-Nederland onder nr. 30244197. Denk s.v.p aan het milieu voor u deze e-mail afdrukt.

------------------------------------------------------------------------------ This message may contain confidential information and is intended exclusively for the addressee. If you receive this message unintentionally, please do not use the contents but notify the sender immediately by return e-mail. University Medical Center Utrecht is a legal person by public law and is registered at the Chamber of Commerce for Midden-Nederland under no. 30244197. Please consider the environment before printing this e-mail.