AxFoundation/strax

`Plugin.__version__` and `Plugin.version` are ambigous

Closed this issue · 3 comments

Plugin.__version__ is used where Plugin.version should be used. Examples:

After writing #690, I'm starting to seriously doubt the usefulness of having a version that depends on the run_id (which isn't even generated from the Plugin.run_id attribute).

Yes I think having run_id dependent versions does not make any sense anymore. To me

strax/strax/plugin.py

Lines 237 to 243 in a0d51fd

def version(self, run_id=None):
"""Return version number applicable to the run_id.
Most plugins just have a single version (in .__version__)
but some may be at different versions for different runs
(e.g. time-dependent corrections).
"""
return self.__version__
looks like some legacy code and with CMT2.0 certainly not required anymore.

But I am wondering if we should not try to be consistent in our code and either use always __version__ or .version(). I would be in favor of the former. If we do the later we might want to make it a property.

But I am wondering if we should not try to be consistent in our code and either use always
Right, this issue is precisely about this 😉

I would also be in favor of dropping the version class method (see my later addendum to this issue). I'm not super thrilled about this PR: #690. I can make #689 still work easily if we drop the .version-method by setting the __getattr__ method appropriately.

My main issue is that the version method can now contain all kinds of weird code, which requires you to init the plugin in order to know what the version is going to be, which is slow. Making it a classmethod would already help.