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

Missing (optional) dependencies #885

Open
fabcor-maxiv opened this issue Mar 18, 2024 · 12 comments · May be fixed by #900
Open

Missing (optional) dependencies #885

fabcor-maxiv opened this issue Mar 18, 2024 · 12 comments · May be fixed by #900

Comments

@fabcor-maxiv
Copy link
Contributor

As a result of investigating this: #874 (comment)

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
Copy link
Contributor Author

fabcor-maxiv commented Mar 18, 2024

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
Copy link
Contributor Author

fabcor-maxiv commented Mar 18, 2024

These are the missing imports I have identified so far:

@fabcor-maxiv
Copy link
Contributor Author

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

@AnnieHeroux
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

Regarding the above list :

  • bliss: Only for BLISS beamlines currently ESRF (optional)
  • devtools: Should in my opinion be (general)
  • grob : Sample changer used by BESSY ? (optional)
  • lima2: So far only ESRF (optional)
  • pyispyb_client: Can be removed
  • pymba: Qt EMBL HH, DESSY ? (optional)
  • qt: Should be removed from mxcuebcore (optional)
  • redis: General IMO
  • SpecClient: Still used by BESSY I think, (optional)
  • tine: EMBL HH, BESSY, DESSY, (optional)
  • v4l2: can be removed
  • vapory: EMBL HH, SOLEIL ? (optional)
  • sardana: ALBA, MAX-IV, (optional)

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

@fabcor-maxiv
Copy link
Contributor Author

@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
Copy link
Contributor

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 (?)

@fabcor-maxiv
Copy link
Contributor Author

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
Copy link
Contributor

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 added a commit to fabcor-maxiv/mxcubecore that referenced this issue Mar 20, 2024
@fabcor-maxiv fabcor-maxiv linked a pull request Mar 20, 2024 that will close this issue
@fabcor-maxiv
Copy link
Contributor Author

I started a draft pull request to experiment with this: #900

fabcor-maxiv added a commit to fabcor-maxiv/mxcubecore that referenced this issue Mar 20, 2024
fabcor-maxiv added a commit to fabcor-maxiv/mxcubecore that referenced this issue Mar 20, 2024
fabcor-maxiv added a commit to fabcor-maxiv/mxcubecore that referenced this issue Mar 20, 2024
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 a pull request may close this issue.

3 participants