pcdshub / lightpath

LCLS Lightpath Module
https://pcdshub.github.io/lightpath
Other
4 stars 9 forks source link

ENH: use graphs for path finding, and apply new Lightpath interface #133

Closed tangkong closed 2 years ago

tangkong commented 2 years ago

Description

This is a doozy, but I think it's time to start writing some of this down.

Motivation and Context

Lightpath in its current incarnation cannot represent the LCLS facility. This representation is hopefully more robust, and allows for new devices and beam paths to be added.

How Has This Been Tested?

Where Has This Been Documented?

This PR. Docstrings and type hints Pre-release notes have been added

Outstanding items

(a hard-to-read visualization of the mock-facility used in the test suite, based roughly on this map )

image
tangkong commented 2 years ago

These tests are fine locally, but there's some mismatch between the happi container and path.json showing up on travis... 😢

tangkong commented 2 years ago

The happi error turned out to be due to the fact that the pip build does not actually install the package, meaning entry points don't get picked up. The test database therefore cannot find the lightpath.happi.containers.LightpathItem container. This was fixed by adding -e ./ to PIP_EXTRAS.

This revealed a failure to tear down QThreads in the pip test suite, which I could not fix despite all the tests passing locally without problem. PIP failures have been allowed, and the python builds limited to 3.9 (latest) to allow for more modern type hinting.

With all that, the tests pass 🎉 Not the most complete victory but a finished one

tangkong commented 2 years ago

I think this is ready for a more final review. I've addressed the previous items and made a slew of to-dos. For the sake of everyone's sanity, it's probably good to end this PR here.

tangkong commented 2 years ago

Thanks for the thorough reviews, I'm going to merge this with the intent to tackle outstanding issues in separate PR's.