What should happen if we start to read without starting the reader?

[thorwhalen]

When revision is made we'll have:

from stream2py.examples.stream_source import SimpleCounterString

source = SimpleCounterString(start=1, stop=10)

reader = source.mk_reader()

with reader:
    print(reader.read())

Creating the reader requires the buffer to start; with reader: just increments a counter in the source to say that there is one active reader

What should happen if the user fails to enter the reader context before trying to read? should it raise an exception?

reader = source.mk_reader()
reader.read()  # should this be an error?

[thorwhalen]

As far as convenience goes, it would be nice to be able to have the reader turn things whenever someone starts to read.

But obviously, there's something really ugly about that.

It's best practice to use context managers, not to manually do a .open() and then a .close().
Here' we'd be encouraging even worse. We'd do the open for the user, and expect them to remember to close?
It seems like it would be setting things up to make nice nooses...

But this convenience is useful as far as easy to use, concern-separated, code.

For example, when using SlabIter, doing something like:

app = SlabIter(
   audio_read=AudioStream(...).read,
   keyboard_read=KeyboardStream(...).read
   # further callables that do stuff with the data
)

SlabIter takes care of turning on anything that has an on switch, and turn everything off when it's over (through exceptions or more natural causes).

The problem here is that .read is a method. It doesn't have an on switch.

So can we get the best of both worlds?

[thorwhalen]

A few solution ideas.

Callable Stream. Have a Stream instance be a callable that points to .read. Why does this scratch the current tradeoff itch? Well, because a Stream instance is a context manage, so can be turned on and off.

Contextualizer (decorator): We could (could we?) make .read return a context manager. That is, still be (sorta) the same callable method, but have an __enter__ and __exit__ that would forward this functionality to the instance it's bound to. Or even better: We could not do that, but rather have contextualize function that would wrap the read to do that. I said "not do that" to be funny. Really, we would solve the "make a (bound) method into a context manager forwarding the context managing to it's instance", but do the right thing and make it a separate, reusable, function. Then we'd have the choice: Do we make all stream readers have thus-decorated read methods, or do we oblige the user to do that explicitly, if and when needed.

I vote for explicit (the latter).

The SlabIter example looks becomes this:

app = SlabIter(
   audio_read=contextualize(AudioStream(...).read),
   keyboard_read=contextualize(KeyboardStream(...).read),
   # further callables that do stuff with the data
)

which is not to bad if you ask me.

[thorwhalen]

Here's some code that implements my wishes above, with tests included.

# ------------------------------------------------------------------------------------------------------------
# The general solution objects

from inspect import signature, Parameter
from abc import abstractstaticmethod
from functools import wraps


class TypeValidationError(TypeError):
    """Raised if an object is not valid"""
    
    @abstractstaticmethod
    def is_valid(obj) -> bool:
        """Returns True if and only if obj is considered valid"""
        
    @classmethod
    def validate(cls, obj, *args, **kwargs):
        if not cls.is_valid(obj):
            raise cls(*args, **kwargs)
    
    
def null_enter_func(obj):
    return obj

def null_exit_func(obj, *exception_info):
    return None

def _has_min_num_of_arguments(func, mininum_n_args):
    return len(signature(func).parameters) >= mininum_n_args

def _callable_has_a_variadic_pos(func):
    """True if and only if the function has a variadic positional argument (i.e. *args)"""
    return any(x.kind == Parameter.VAR_POSITIONAL for x in signature(func).parameters.values())

class EnterFunctionValidation(TypeValidationError):
    """Raised if an function isn't a valid (context manager) __enter__ function"""
    @staticmethod
    def is_valid(obj):
        return callable(obj) and _has_min_num_of_arguments(obj, 1)
    

    
class ExitFunctionValidation(TypeValidationError):
    """Raised if an function isn't a valid (context manager) __exit__ function"""
    @staticmethod
    def is_valid(obj):
        return callable(obj) and (_callable_has_a_variadic_pos or _has_min_num_of_arguments(obj, 4))
    
# TODO: If needed: 
#  A more general Contextualizer would proxy/delegate all methods (including special ones) to the wrapped
class ContextualizeCall:
    def __init__(self, func, enter_func=null_enter_func, exit_func=null_exit_func):
        # validation
        if not callable(func):
            raise TypeError(f"First argument should be a callable, was: {func}")
        EnterFunctionValidation.validate(
            enter_func, 
            f"Not a valid enter function (should be a callable with at least one arg): {enter_func}"
        )
        ExitFunctionValidation.validate(
            exit_func, 
            f"Not a valid exit function (should be a callable with at least one arg or varadic args): {exit_func}",
        )
        # assignment
        self.func = func
        self.enter_func = enter_func
        self.exit_func = exit_func
        # wrapping self with attributes of func (signature, name, etc.)
        wraps(func)(self)
        
    def __call__(self, *args, **kwargs):
        return self.func(*args, **kwargs)
        
    def __enter__(self):
#         print(f'{type(self).__name__}.__enter__')
        return self.enter_func(self.func)
    
    def __exit__(self, *exception_info):
#         print(f'{type(self).__name__}.__exit__')
        return self.exit_func(self.func, *exception_info)
    

# ------------------------------------------------------------------------------------------------------------
# Using these general objects for the particular case of having bound methods forward their context
# management to the instances they're bound to

from functools import partial

def forward_to_instance_enter(obj):
    return obj.__self__.__enter__()

def forward_to_instance_exit(obj, *exception_info):
    return obj.__self__.__exit__(obj, *exception_info)

contextualize_with_instance = partial(
    ContextualizeCall, 
    enter_func=forward_to_instance_enter, 
    exit_func=forward_to_instance_exit
)
contextualize_with_instance.__doc__ = (
    "To be applied to a bound method. "
    "Returns a callable that forwards context enters/exits to the bound instance"
)


# ------------------------------------------------------------------------------------------------------------
# Testing this 

# Test objects

class StreamHasNotBeenStarted(RuntimeError):
    """Raised when an action requires the stream to be 'on'"""
    
class Streamer:
    def __init__(self, iterable):
        self.iterable = iterable
        self.is_running = False
        self._read = None
        
    def enter(self):
#         print(f'{type(self).__name__}.enter')
        self._read = iter(self.iterable).__next__
        self.is_running = True
        
    def exit(self, *exc):
#         print(f'{type(self).__name__}.exit')
        self._read = None
        self.is_running = False
        
    __enter__, __exit__ = enter, exit
        
    def read(self):
        if not self.is_running:
            raise StreamHasNotBeenStarted(
                "The stream needs to be on/started (in a context) for that!"
            )
        return self._read()
    
    
# Testing the test objects

s = Streamer('stream')

# demo
with s:
    assert s.read() == 's'
    assert s.read() == 't'
    
# a reader test function
def test_reader(reader):
    assert ''.join(reader() for _ in range(6)) == 'stream'

# Normal case: With a context

reader = s.read

with s:
    test_reader(reader)
    
# Normal case: Manual entering/exiting

s.enter()
reader = s.read
test_reader(reader)
s.exit()


# But if we don't turn things on...

reader = s.read
try:
    # oops, forgot the enter s context
    test_reader(reader)
    it_worked = True
except StreamHasNotBeenStarted as e:
    it_worked = False
    assert isinstance(e, StreamHasNotBeenStarted)
    assert e.args[0] == 'The stream needs to be on/started (in a context) for that!'
assert not it_worked


# But we can't turn the read method on -- it's not a context (it's instance is!)

reader = s.read

try:
    with reader:  # can we actually do this (answer: no! We can enter s, not s.read)
        test_reader(reader)
    it_worked = True
except Exception as e:
    it_worked = False
    assert isinstance(e, AttributeError)
    assert e.args[0] == '__enter__'  # well yeah, reader doesn't have an __enter__!
    
assert not it_worked


    
# But with contextualize_with_instance...

reader = contextualize_with_instance(s.read)

try:
    with reader:  # now we can enter the reader!
        test_reader(reader)
    it_worked = True
except Exception as e:
    it_worked = False

    
assert it_worked  # Hurray!

[andeaseme]

@thorwhalen I'm a little surprised you'd vote for explicit instead of always doing it in the background since you're an advocate for reduced overhead.
I'm in favor of always decorating because it won't change other use cases and were gravitating toward using SlabIter more.

[thorwhalen]

@andeaseme: I'm an advocate for explicit over implicit by default, and as long as it's not in the way of usage too much.

(The illusion of the opposite opinion might come from the fact that have low tolerance for difficult/unclear usage.)

[thorwhalen]

I now implemented and pushed my solution in stream2py.
Some doctests and tests included.
It's completely independent (neither dependent on nor dependent... under (?) anything in stream2py), so nice and safe.

I'll leave the issue open until you and @SRHerzog vet the solution, and then we can close the issue.

See the code here: https://github.com/i2mint/stream2py/blob/d5ba8fdd1718ca084831a6b97a9698c111caa10a/stream2py/util.py

And the tests here:
https://github.com/i2mint/stream2py/blob/d5ba8fdd1718ca084831a6b97a9698c111caa10a/stream2py/tests/test_util.py

[thorwhalen]

Again, this is where the "problem" arose first, but I depend on nothing in stream2py, nor does stream2py depend on it.
We might find that this util is more widely applicable, and can then move where it makes sense (i2? creek?).

On Monday, let's discuss also removing what ever we did to have those automatic context entering.

[thorwhalen]

And @andeaseme , about

I'm a little surprised you'd vote for explicit instead of always doing it in the background since you're an advocate for reduced overhead.

I might be for doing it in the background here if it didn't have the downsides mentioned above.

If you have an idea that in at the intersection of the best of both worlds, let us know!

[thorwhalen]

If you haven't read the solution yet, know that the core of the solution is simply:

from functools import wraps

class ContextualizeCall:
    def __init__(self, func, enter_func, exit_func):
        self.func, self.enter_func, self.exit_func = func, enter_func, exit_func
        # wrapping self with attributes of func (signature, name, etc.)
        wraps(func)(self)
        
    def __call__(self, *args, **kwargs):
        return self.func(*args, **kwargs)
        
    def __enter__(self):
        return self.enter_func(self.func)
    
    def __exit__(self, *exception_info):
        return self.exit_func(self.func, *exception_info)

It was the original code -- I just expanded on that the code to make it more production-ready (validation, etc.)

[thorwhalen]

Done: StreamBuffer.mk_reader() (stream2py/stream_buffer.py:193) checks isinstance(self.source_buffer, _SourceBuffer) and raises StreamNotStartedError with an actionable message if the buffer hasn't been started. Landed in commit cc0b18c (merged via PR #21) but never closed on GitHub. Closing now.