ben-denham / labtech

Easily run experiment permutations with multi-processing and caching
https://ben-denham.github.io/labtech/
GNU General Public License v3.0
7 stars 1 forks source link

Add tests for `task` #11

Closed nathanjmcdougall closed 4 months ago

nathanjmcdougall commented 5 months ago

Lots of unit tests.

Two are failing. _lt is a reserved keyword, but it is also being used as a way to detect classes which are tasks - and we don't raise errors about reserved attributes in such cases. Is this just for if we are inheriting from a previously defined Task class? I wonder whether there is a better way to do this (perhaps by inspecting directly the MRO), because I think we still want to protect the user if they accidentally use _lt as an attribute.

Keen to know what you'd do about this.

nathanjmcdougall commented 5 months ago

Need to pass the linter

ben-denham commented 5 months ago

These tests look great thanks, and good catch with _lt. You're correct that the is_task_type() guard around attribute checking is for handling inheritance (that probably deserves a comment). I think we should handle this by making is_task_type() more robust (as well as is_task() while we're at it):

def is_task_type(cls):
    """Returns `True` if the given `cls` is a class decorated with
    [`labtech.task`][labtech.task]."""
    return isclass(cls) and isinstance(getattr(cls, '_lt', None), TaskInfo)

def is_task(obj):
    """Returns `True` if the given `obj` is an instance of a task class."""
    return is_task_type(type(obj)) and hasattr(obj, '_is_task')