Presets, Plugin Updates and JCAMP-DX

[mzieg]

I probably should have broken this up into more PRs.

• plugins
  • removed plugin 'dependencies'; increase default plugin Marquee period
  • support events in functional plugins
  • provide self.marquee_message for functional plugins
  • add combobox datatype for plugin fields
  • allow plugin fields to have stylesheets
• file_io
  • save as JCAMP-DX
  • support loading columnar CSV files which don't happen to include 'Integration Time'
• added PresetFeature
• refactoring
  • moved RamanIntensityCorrection, ScanAveragingFeature, BaselineCorrection, BoxcarFeature, IntegrationTimeFeature to ctl
  • docs and cleanup around ScrollStealFilter and MouseWheelFilter
  • simplified Configuration.save

[samiebee43]

Yes this is kind of a lot to review at once, but most of it looks good.

Two things stand out to me.

1. Deprecation of plugin dependencies

Appears to be a duplicate of #311 . This can be handled a number of ways, but it may be worthwhile to diff the two branches to see if they differ more than expected. One way to do this would be.

# should contain the largest set of filenames
git diff --name-only mzieg-followup-testing 4.1.0-dev

# should contain a strict subset of filenames, plugin related
git diff --name-only mzieg-4.1.0-plugin-features 4.1.0-dev

# should contain a strict subset of filenames, not plugin related
git diff --name-only mzieg-followup-testing mzieg-4.1.0-plugin-features

Overall, it's a non-issue. If both branches are good, then they can both be merged in any order and the whole thing should be fine, but it's a bit curious that such an overlap happened.

2. Preset Feature API

There's a nice comment that explains the preset feature's API.

class IntegrationTimeFeature:
    def __init__(self):
        ctl.presets.register(self, ["integration_time_ms"])
    def get_preset(self, attr):
        if attr == "integration_time_ms":
            return self.current_ms
    def set_preset(self, attr, value):
        if attr == "integration_time_ms":
            self.set_ms(int(value))

This looks pretty good, it handles situations where we need to do more than update a variable--for example modifying the GUI to match. This is not the soln I had in mind, but that's okay. The main thing I would suggest is to avoid passing attr's, and to explicitly state callbacks in the registration step. Also, avoid passing an array to register--rather call it multiple times.

class IntegrationTimeFeature:
    def __init__(self):
        ctl.presets.register(self, "integration_time_ms", get_integration_time, set_integration_time_ms)

    def get_integration_time_ms(self):
        return self.current_ms

   def set_integration_time_ms(self, value):
       value = int(value)
       # existing definition ... 

For completeness, the solution I had in mind would be to use resolution and search-paths. This would involve creating a parameter construct which represents the kinds of things that we might want to preset, persist, etc. Classes that depend on Enlighten Parameters access those values via a third party every time (kind of like how we access spec from multispec). Setting the value also uses the same third party. The parameter access routine will have some logic to decide whether to load the value from the .ini file, eeprom, a hardcoded default, or from the active preset. It requires that upon selecting a preset, that the GUI is updated to call in new resolutions for each parameter. Parameters perform validation and correction at access time.

class IntegrationTimeFearture:
    def __init__(self):
        pass

    def get_integration_time_ms(self):
        # update GUI if necessary
        # ...

        # get the parameter value, whether it's driven by config, preset, eeprom, etc
        return EnlightenParameters.resolve("integration_time_ms")
        

    def set_integration_time_ms(self, value):
         # save the parameter value, storing changes in config or preset as appropriate.
         EnlightenParameters.save("integration_time_ms", value)

         # update GUI if necessary
         # ...

Upon enabling a preset, we would need to make sure IntegrationTimeFeature is updated by calling get_integration_time_ms to get the latest decision from EnlightenParameters.

[samiebee43]

If this works we should merge it.

There are a lot of helpful changes that makes this seem like it should be the latest point in revision history.

---

Update, test results

• Settings Presets section working
• Local Baseline plugin working
• Writing jcamp failed

I got jcamp using pip install jcamp instead of rebuilding my entire env... might that be the cause?

[samiebee43]

check jcamp save

[mzieg]

check jcamp save

Can you expand on that? I have definitely saved many .dx files successfully...but that may be because I have ..\jcamp in my PYTHONPATH.

I'll add it to requirements.txt.

[mzieg]

I'll add it to requirements.txt.

Actually...we should probably fork the version of jcamp that has writefile, and add it as a manual dependency...hmm.

[mzieg]

class IntegrationTimeFearture:

Nice Halloween pun :-)

[mzieg]

The main thing I would suggest is to avoid passing attr's, and to explicitly state callbacks in the registration step. Also, avoid passing an array to register--rather call it multiple times.

Updated.

[samiebee43]

Thank you for including that suggestion.

Note: Not that this particularly matters, but the spelling is getter/setter not gettor/settor.

[samiebee43]

OK. I was wrong, we should probably fork this and merge frenchy's branch to main on our fork.

[mzieg]

WasatchPhotonics/jcamp#1

[mzieg]

getter/setter not gettor/settor

TIL