Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAINT: Add pint as a requirement #592

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

larsoner
Copy link

@larsoner larsoner commented Oct 5, 2023

On my system pip intstall magicgui followed by running a variant of the "all of these" example from https://pyapp-kit.github.io/magicgui/widgets/#magicgui I got:

monolith:Desktop larsoner$ python -ui magic.py 
Traceback (most recent call last):
  File "/Applications/MNE-Python/1.5.0_1/.mne-python/lib/python3.11/site-packages/superqt/spinbox/_quantity.py", line 4, in <module>
    from pint import Quantity, Unit, UnitRegistry
ModuleNotFoundError: No module named 'pint'

The above exception was the direct cause of the following exception:

...
  File "/Applications/MNE-Python/1.5.0_1/.mne-python/lib/python3.11/site-packages/superqt/__init__.py", line 57, in __getattr__
    from .spinbox._quantity import QQuantity
  File "/Applications/MNE-Python/1.5.0_1/.mne-python/lib/python3.11/site-packages/superqt/spinbox/_quantity.py", line 7, in <module>
    raise ImportError(
ImportError: pint is required to use QQuantity.  Install it with `pip install pint`

I guess pint is an optional dependency for superqt (?) but it seems like magicgui should require it to make sure all examples work out of the box.

@tlambert03
Copy link
Member

hey @larsoner 👋 nice to see you here and thanks for opening a PR.

I'm hesitant to add more base level dependencies to magicgui. By its nature, magicgui might potentially support many third party types (beyond pint) but shouldn't require those packages for installation. My assumption with pint was that, if someone wants to provide a widget representation of a pint.Quantity, then that user would be declaring a dependency on pint already. We do add a quantity extra (pip install magicgui[quantity]) along with a number of other optional types:

magicgui/pyproject.toml

Lines 56 to 66 in 21ee452

pyqt5 = ["pyqt5>=5.12.0"]
pyqt6 = ["pyqt6"]
pyside2 = [
"pyside2>=5.14 ; python_version=='3.8'",
"pyside2>=5.15 ; python_version>='3.9'",
]
pyside6 = ["pyside6"]
tqdm = ["tqdm>=4.30.0"]
jupyter = ["ipywidgets>=8.0.0"]
image = ["pillow>=4.0"]
quantity = ["pint>=0.13.0"]

I see that this means that someone can't immediately run all of the examples without additional installations, but if we add pint, then a similar argument could be made for pillow, tqdm, and any number of third party types that we're able to support.

As an alternative, would you support the addition and of an all or examples extra that brings in all of these optional dependencies for someone wanting to run all of the examples?

@larsoner
Copy link
Author

larsoner commented Oct 5, 2023

would you support the addition and of an all or examples extra

Actually I didn't even notice those options. Maybe just a .. note:: or .. warning:: where the QuantityEdit shows up on that examples page and also the QuantityEdit API page would help to know about when optional dependencies are needed. This would also be relevant for the matplotlib stuff we've been discussing.

Just let me know if you agree with how and where these should be documented (or if you have other ideas) and I'm happy to adjust this PR to be a documentation update instead 👍

@tlambert03
Copy link
Member

Just let me know if you agree with how and where these should be documented (or if you have other ideas) and I'm happy to adjust this PR to be a documentation update instead

agree!
A note could be added to the doc header of this file and also to the docstring here.

i just tried locally, and mkdocs admonition syntax (!!note) doesn't appear to be working in the gallery pages at the moment. so if you just make a simple note i can fix the admonitions later

@tlambert03
Copy link
Member

(note also: I just killed the antiquated setup.py file in #595 ... so your conflict is due to that. it can just be removed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants