#44308 closed defect (invalid)
py-SRPAstro: switch PIL/Pillow dependency to py-matplotlib and py-scipy.
Reported by: | Ionic (Mihai Moldovan) | Owned by: | petrrr |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | |
Keywords: | haspatch | Cc: | stefano.covino@…, seanfarley (Sean Farley), michaelld (Michael Dickens) |
Port: | py-SRPAstro py-matplotlib py-scipy |
Description
py-SRPAstro
has a (formal) dependency on PIL
, although it doesn't use PIL
in its code.
However, PIL
is needed by both py-matplotlib
and py-scipy
-- dependencies of py-SRPAstro
-- which currently, wrongly do not declare it as a dependency.
The author has confirmed in private mail, that not his software (py-SRPAstro
) is requiring/using PIL
, but indeed the other two ports.
Thus, switch over the PIL
/Pillow
dependency (in its path:
-based form as suggested/worked on in #44285) from py-SRPAstro
to py-matplotlib
and py-scipy
.
Attachments (1)
Change History (12)
Changed 10 years ago by Ionic (Mihai Moldovan)
Attachment: | py-SRPAstro-py-matplotlib-py-scipy-pil-dep-move.patch added |
---|
comment:1 Changed 10 years ago by Ionic (Mihai Moldovan)
Revbumped ports to account for dependency change.
comment:2 Changed 10 years ago by petrrr
openmainter
should be added to py-SRPAstro
, waiting for confirmation by the maintainer.
comment:3 Changed 10 years ago by petrrr
I just looked though the code. It specifies pil as an requirement, so I would argue that at least formally it depends on PIL. So this should be better addressed upstream.
sudo grep -i pil * -r [...] SRPAstro.egg-info/requires.txt:pil setup.py: install_requires=['scipy', 'astlib>=0.4', 'pil', 'matplotlib', 'atpy',
comment:4 Changed 10 years ago by petrrr
I do not see why py-scipy
should declare a dependency on PIL or Pillow, but I found the following:
scipy/misc/pilutil.py:Note that PIL is not a dependency of SciPy and this module is not scipy/misc/pilutil.py:available on systems that don't have PIL installed.
If you still think it should, please document why!
comment:5 Changed 10 years ago by petrrr
For py-matplotlib
there seems to be no strict requirement for PIL neither. Import are found only in lib/matplotlib/backend_bases.py
, lib/matplotlib/image.py
and lib/matplotlib/tests/test_image.py
, but the absence of PIL is handled smoothly. All other occurrences are only in documentation or examples.
So the missing dependence on PIL seems to be intentional.
From lib/matplotlib/backend_bases.py
:
try: from PIL import Image _has_pil = True except ImportError: _has_pil = False _backend_d = {}
From lib/matplotlib/image.py
def pilread(fname): """try to load the image with PIL or return None""" try: from PIL import Image except ImportError: return None
From lib/matplotlib/tests/test_image.py
:
try: from PIL import Image HAS_PIL = True except ImportError: HAS_PIL = False
comment:6 Changed 10 years ago by petrrr
Cc: | petr@… removed |
---|---|
Owner: | changed from macports-tickets@… to petr@… |
Status: | new → assigned |
comment:7 Changed 10 years ago by petrrr
After reviewing this it looks like the dependencies of these ports are currently handled correctly.
The PIL dependency should stay with py-SRPAstro
. If you still believe this port should not depend on PIL
, please report upstream.
comment:8 Changed 10 years ago by petrrr
See ticket #44376, for change path:
-based PIL
/Pillow
dependency, along with other dependency corrections.
comment:9 Changed 10 years ago by petrrr
Resolution: | → invalid |
---|---|
Status: | assigned → closed |
comment:10 Changed 10 years ago by Ionic (Mihai Moldovan)
scipy
:
/opt/local/var/macports/build/_opt_macports_python_py-scipy/py26-scipy/work/scipy-0.14.0/scipy/misc/pilutil.py: from PIL import Image, ImageFilter /opt/local/var/macports/build/_opt_macports_python_py-scipy/py26-scipy/work/scipy-0.14.0/scipy/misc/pilutil.py: import Image /opt/local/var/macports/build/_opt_macports_python_py-scipy/py26-scipy/work/scipy-0.14.0/scipy/misc/pilutil.py: import ImageFilter /opt/local/var/macports/build/_opt_macports_python_py-scipy/py26-scipy/work/scipy-0.14.0/scipy/ndimage/io.py: from PIL import Image
Yes, PIL
is an optional dependency for both scipy
and matplotlib
, but I think it's highly recommended to have the modules around for having a complete feature set.
Maybe adding PIL
/Pillow
as a hard dependency is wrong, but it may certainly warrant a variant (to my mind even turned on by default.)
SRPAstro
does formally depend on PIL
, but only due to scipy
/matplotlib
. I haven't checked exactly, but I figure it uses classes from scipy
/matplotlib
which are only available with PIL
. The author confirmed this.
comment:11 Changed 10 years ago by petrrr
I really believe the current approach is appropriate.
Intentionally neither matplotlib
nor scipy
declare a dependency on PIL
, probably for some good reason (I could imagine to avoid any of the fuss with PIL), so MP should neither.
Adding a variant, does not provide any advantage, it just complicates the situation while installing exactly the same set of files. You cannot guaranty the right variant is installed anyway, but you can do this for PIL
/Pillow
. The extra functionally provided seems not to be expected by users or software (no tickets), and if you need it you just have to install PIL
or Pillow
, or just uninstall any time you want. So I see no problem here.
SRPAstro
does exactly this, it expects this extra functionality provided by PIL, so it declares the dependency on it, and only this can guaranty it is around, with a variant you cannot be sure.
Anyway, if you disagree feel free to reopen, but in this case I would leave the decision to the maintainers of the ports.
Move
PIL
/Pillow
dependency frompy-SRPAstro
topy-matplotlib
andpy-scipy
. Revbump.