WasatchPhotonics / ENLIGHTEN

Open-source spectroscopy application for controlling and taking measurements from Wasatch Photonics spectrometers.
https://wasatchphotonics.com/product-category/software/
MIT License
3 stars 6 forks source link

Sb roi handles #337

Closed samiebee43 closed 9 months ago

samiebee43 commented 10 months ago

Attempting to save spectra on mzieg-roi-handles created an error message. This happens because Measurement is relying on Multispec to carry a reference to Graph. The solution was to give Measurement a ctl parameter so it can reference Graph directly.

With this comes some more refactoring. Any class that depends on a live instance takes a ctl parameter, including ColumnFileParser which depends on a save_options and a measurements instance. Self contained classes such as DashFileParser do not need (and should not have) the ctl parameter.

image

Measurment gets a ctl parameter as well as some other parameters. Only the save_options and measurements fields were removed. MeasurementFactory keeps some of it's copied references not because that's the ideal state, but to limit the scope of this PR. That class has ctl tacked along with the other parameters (redundant).

Remember any time you inherit from an object you can simply omit inheritance for the exact same meaning. Object is inherited by default.

mzieg commented 10 months ago

Any interest in just giving Controller a static get_instance() method so that any code in ENLIGHTEN can just grab their own Controller instance anytime they want, and we won't need to keep passing it around?

Good refactor. Apologies for trying to merge code that clearly wasn't ready.

samiebee43 commented 9 months ago

Yes that would be interesting.

One possible downside is that no longer would classes be identified as self-contained or not. For instance ColumnFileParser is not self contained because it takes a ctl param and depends on some Enlighten instances, but DashFileParser is self contained and can be trivially copied to another program.

But generally yes, in fact that would make Enlighten more compliant with Enlighten Extended Development section 4.4.