Opened 3 years ago
Closed 3 years ago
#7173 closed enhancement (fixed)
RFE: Get optional dependencies from the Toolshed and/or pip
| Reported by: | Zach Pearson | Owned by: | Zach Pearson |
|---|---|---|---|
| Priority: | moderate | Milestone: | |
| Component: | Infrastructure | Version: | |
| Keywords: | Cc: | chimerax-programmers | |
| Blocked By: | Blocking: | ||
| Notify when closed: | Platform: | all | |
| Project: | ChimeraX |
Description
Tom notes that the ChimeraX distribution is growing large. I don't disagree.
It should be possible to mark some part of a bundle's functionality as depending on a package that's not included in the distribution. Other python packages have solved this somewhat gracefully with flags in code:
_has_dep = False
try:
import optional_dep
_has_dep = True
except ImportError:
_has_dep = False
...
# Code that warns the user if they hit a codepath that uses the dependency.
But I prefer a decorator:
def uses_optional(dep, func):
try:
import dep
return func
except ImportError:
def warn_and_attempt_install_instead():
... # Warn the user they need to install something, or prompt them with a popup and with consent try to install it from the toolshed
return warn_and_attempt_install_instead
Tom noted that the ChimeraX-side code isn't the difficult part; that'll be making the Toolshed support it.
I agree we should do a trial run with the DICOM bundle.
Change History (8)
comment:1 by , 3 years ago
comment:2 by , 3 years ago
Sorry, I know we'll be discussing this Monday but I just can't help myself. :-). This situation is exactly what import hooks are for, and they provide a seamless solution where the programmer does not have to write a whole bunch of additional code to handle a missing package. If ChimeraX knows how to get the bundle then it handles the situation. Conversely, the proposed solutions are bad in that catching ImportError does not in fact in any way guarantee that the bundle isn't already installed. The bundle may be in turn importing something (such as gdcm) that is causing the ImportError. To ensure that you aren't mystifying the user by telling them to install a bundle that they've already installed, you would have to scan sys.modules to ensure that the bundle is in fact missing -- which is what an import hook does for you automatically!
comment:3 by , 3 years ago
No one is questioning the import hook. The DICOM bundle should be complete. Any file that the DICOM bundle can't open should log a message and move on. You don't need to support all variations of DICOM files, even if you know how. It's not worth your time nor the bytes. If we had a collaborator that needed the functionality, then add it.
The only Python packages that ChimeraX should offer to install are bundles from the Toolshed. That is our deliverable.
comment:4 by , 3 years ago
I would suggest it is better to start with a ticket that specifically tries to solve this problem for the DICOM / lossless jpeg case. I don't think it is very productive to immediately try for a general solution when not even a single case of the problem has been handled. It is too hard to get it right until after maybe the 2nd or 3rd case of this problem has been solved. At any rate, that is just philosophy, and the person who owns the ticket can decide the approach.
This ticket needs an owner. Zach, our usual practice is to assign an owner when we make a ticket. If it is left as owner none it is an invitation to nothing happening with no one responsible. I make a lot of tickets my simple rule is I assign them to myself unless I think it is clearly someone else's problem, or unless I have asked if someone else wants to be the owner. So maybe assign it to yourself, and then if you want to make someone else the owner ask them if they agree and reassign.
comment:5 by , 3 years ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
Sure, I'll assign this ticket to myself, that's fair.
I can't speak for anyone else, but when I prototype solutions I tend to start with simple cases and work up to a general case. I mean this with maximal respect, but I'm not an intern, and I don't need to be told how to be effective. If I need my work reviewed, I'll ask for it, otherwise you really don't need to look over my shoulder as much as you currently do.
comment:6 by , 3 years ago
Do we consider this closed now that we can pip install in ChimeraX and TOML-based bundles can note optional dependencies? We only need to gracefully tell the user what to do when they hit a code path with an optional dependency.
follow-up: 7 comment:7 by , 3 years ago
You can close the ticket if you want. You both made the ticket and own it so you decide when it is done. It is rather general and I think in the future we will have specific cases where we want to defer the installation of a PyPi package until the user actually needs it. When that happens we can make a new ticket for that specific case.
comment:8 by , 3 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Just wanted to give everyone the opportunity to object to its closure, since it affects everyone.
Since the initial work is done further refinements can come from future tickets referring to this one.
Previous discussion can be found in #1745.