search

mxcube / mxcubecore

Backend used by MXCuBE
http://mxcube.github.io/mxcube/
GNU Lesser General Public License v3.0
11 stars 51 forks source link

Missing (optional) dependencies #885

Open fabcor-maxiv opened 3 months ago

fabcor-maxiv commented 3 months ago

As a result of investigating this: https://github.com/mxcube/mxcubecore/pull/874#issuecomment-2003211779

I now realize there are a bunch of imports in the code base that are not associated with a dependency in pyproject.toml.

As a side effect it triggers a whole bunch of errors and warnings when build the documentation (the auto-generated API part of the documentation).

Maybe we can fix that.

fabcor-maxiv commented 3 months ago

I suggest we add those dependencies as optional dependencies in pyproject.toml.

I am thinking we could also group them by facility and/or beamline.

So we could have optional groups of dependencies (also called extras) such as maxiv-biomax (would contain PyTango), esrf, desy, and so on.

But I need help with that, to figure out what facility is running what exactly, especially which versions so that they can be added to pyproject.toml and the lockfile with the correctly pinned version.

cc: @marcus-oscarsson

fabcor-maxiv commented 3 months ago

These are the missing imports I have identified so far:

fabcor-maxiv commented 3 months ago

Then maybe all those imports could/should be wrapped in a try block, in case it helps (I am not convinced, yet).

AnnieHeroux commented 3 months ago

Some facilities/beamline might be using a mix, so having specific groups of dependencies would be tedious. I am fairly certain that beamlines using Epics don't mix with Pytango/Taco/Sardana but they might use Exporter .... So far it seems to only impact the generation of the documentation.

marcus-oscarsson commented 3 months ago

As @AnnieHeroux pointed out its not uncommon that there is a mix of the various dependencies and that many of these are not exclusive to only one site. In that case each site has to maintain their section in the pyproject.toml file. So far, each site installs the dependencies they need (that are used in their site specific files) independently.

Maybe adding a section that is maintained by each site makes this work easier ?

I think its important that we keep things as straightforward as possible, it would probably not help if one needs to pass tons of things to --with.

For the try/except in general I would agree but in this case I would prefer that we get an exception for missing dependencies in site specific code. I think we should use try/except in non site specific code but then I would consider the dependency to be general.

fabcor-maxiv commented 3 months ago

Some facilities/beamline might be using a mix, so having specific groups of dependencies would be tedious.

@AnnieHeroux, I do not know that there would be a problem here. Each facility can maintain their own list of optional dependencies for themselves if they want to. And we can have some other list(s) that are based by topic.

Something like this (probably not this exact notation, I still need to figure this out):

[tool.poetry.extras]
bliss = ["bliss"]
pymba = ["pymba"]
redis = ["redis"]
sardana = ["sardana"]
tango = ["PyTango"]
v4l2 = ["v4l2"]
vapory = ["Vapory"]

maxiv = ["mxcubecore[sardana,tango]"]

In this example, maxiv is an optional group (an extra) that refers to other optional groups. But it could also be expressed like in the following:

maxiv = ["PyTango", "sardana"]

Goal is to have all dependencies clearly listed somewhere (in pyproject.toml if possible), so that one is not left scratching their head figuring out where a missing import comes from. Seems like the right thing to do, to me.

marcus-oscarsson commented 3 months ago

Regarding the above list :

A problematic details will be that each site (or even beamline) might want to use different versions of the same dependency,

fabcor-maxiv commented 3 months ago

@marcus-oscarsson Thanks for the list :)

A problematic details will be that each site (or even beamline) might want to use different versions of the same dependency,

Yes, I am also worried about this. But maybe there is a compromise to be made somewhere. We can investigate.

If you know which version ranges ESRF needs/wants for their optional dependenices, I can slowly start building this. We'll see how far we can get before things start to break (version conflicts).

AnnieHeroux commented 3 months ago

This one goes unnoticed because it was already in a try block in the module.

fabcor-maxiv commented 3 months ago

This one goes unnoticed because it was already in a try block in the module.

* epics: most facilities in America (North and South), Australia, Asia (?)

Thanks @AnnieHeroux, I added to my list. By chance, do you know what needs to be installed for the Python import to work, is it a package available on PyPI or conda?

AnnieHeroux commented 3 months ago

i tried pip install pyEpics then tested by removing the block for the import epics in the Epics.py module using make html was successful at showing the module.

Matt thinks many facility/beamlines may do their own thing about epics import (conda or Pypi or ???)

fabcor-maxiv commented 3 months ago

I started a draft pull request to experiment with this: https://github.com/mxcube/mxcubecore/pull/900