bluesky / ophyd-async

Hardware abstraction for bluesky written using asyncio
https://blueskyproject.io/ophyd-async
BSD 3-Clause "New" or "Revised" License
12 stars 26 forks source link

Move FastCS Eiger to `fastcs`, move odin io/writer to seperate modules outside of eiger #557

Open jwlodek opened 2 months ago

jwlodek commented 2 months ago

Looking at the recent fastcs eiger support, I noticed two things:

epics/eiger/_odin_io.py - This appears to be general to all Odin detectors, is that correct? If yes, I think it should be decoupled from eiger, maybe in fastcs/odin/_odin_io.py and fastcs/odin/_odin_hdf_writer.py or something similar.

Also, should the eiger support be moved under fastcs since it is a fastcs implementation? Then a separate ADEiger version could be placed under epics.

Thoughts?

DominicOram commented 2 months ago

Despite doing the work I have no strong opinions on it's organisation. I defer to @coretl and @GDYendell who know more about the bigger picture than I do, I would just like to get a working ophyd-async eiger...

coretl commented 2 months ago

In the epics module we take the support module name and PEP8 it, so we have epics/adaravis. Could we do the same with the FastCS modules? Would that be fastcs/odin and fastcs/eiger, or are the repos named something different? @GDYendell what do you think?

GDYendell commented 2 months ago

I think this sounds reasonable.

The repos are currently eiger-fastcs and odin-fastcs. It seems the convention on PyPI package naming is actually to have the core module name at the start, e.g. pytest-*, so maybe we should change that?

coretl commented 2 months ago

I think this sounds reasonable.

The repos are currently eiger-fastcs and odin-fastcs. It seems the convention on PyPI package naming is actually to have the core module name at the start, e.g. pytest-*, so maybe we should change that?

That probably makes sense...

Then we have fastcs/odin that comes from fastcs-odin. Looks neat

GDYendell commented 2 months ago

I have renamed things: