Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document and test direct use of stdio_mgr, not as a context manager #38

Closed
bskinn opened this issue Aug 5, 2019 · 7 comments
Closed
Labels
doc Changes/additions to the project documentation maybe This may or may not be implemented needs tests Tests are absent or incomplete for this
Milestone

Comments

@bskinn
Copy link
Owner

bskinn commented Aug 5, 2019

jayvdb:

Also since you mentioned __enter__. We should have some tests and documented behaviour for using stdio-mgr without with. i.e. on Py37

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot unpack non-iterable _GeneratorContextManager object
>>> m = stdio_mgr(close=False)
>>> m
<contextlib._GeneratorContextManager object at 0x7f7d6b8d3be0>
>>> dir(m)
['__abstractmethods__', '__call__', '__class__', '__delattr__', '__dict__', '__dir__', '__doc__', '__enter__', '__eq__', '__exit__', '__format__', '__ge__', '__getattribute__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__module__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__slots__', '__str__', '__subclasshook__', '__weakref__', '_abc_impl', '_recreate_cm', 'args', 'func', 'gen', 'kwds']
>>> list(m.gen)
[(TeeStdin(tee=<_io.TextIOWrapper encoding='utf-8'>, init_text='', _encoding='utf-8'), <_io.TextIOWrapper encoding='utf-8'>, <_io.TextIOWrapper encoding='utf-8'>)]
>>> m = stdio_mgr(close=False)
>>> (i, o, e) = list(m.gen)[0]
>>> i
TeeStdin(tee=<_io.TextIOWrapper encoding='utf-8'>, init_text='', _encoding='utf-8')
>>> o
<_io.TextIOWrapper encoding='utf-8'>
>>> e
<_io.TextIOWrapper encoding='utf-8'>
>>> o.buffer
<_io.BufferedRandom>
>>> o.buffer.raw
<stdio_mgr.stdio_mgr._PersistedBytesIO object at 0x7f7d6b985200>

If we did change stdio_mgr to a class, the resulting class would be like a text fifo between i and o?
How can it be useful? They can access it anyway using the above loop-hole.
This might also help when trying to come up with a class name for it, if the function is to be replaced with a class.
Or can we close the loop-hole, unless moving to a class.

If someone really wants to use it this way, they probably know enough about context managers and Python objects that they don't need stdio_mgr converted into a class (cf. #10). But, if using the machinery this way enables significant/valuable new/different/unexpected usage of stdio_mgr generally, then it's probably worth at least documenting & testing it, even if it doesn't warrant going all the way to #10.

@bskinn bskinn added maybe This may or may not be implemented doc Changes/additions to the project documentation needs tests Tests are absent or incomplete for this labels Aug 5, 2019
@bskinn bskinn added this to the Future milestone Aug 5, 2019
@jayvdb
Copy link
Contributor

jayvdb commented Aug 26, 2019

I guess this is now mostly redundant due to #53 being merged?

@bskinn
Copy link
Owner Author

bskinn commented Aug 26, 2019 via email

@jayvdb
Copy link
Contributor

jayvdb commented Aug 26, 2019

I am unable to close it.

@bskinn
Copy link
Owner Author

bskinn commented Aug 26, 2019

Mmph. Limitations of collaboration on a personally-held repo, I expect.

Your vision on stdio-mgr is considerably broader and more comprehensive than mine. My major desire is for it to keep serving my use-cases, which are a ~tiny subset of what you're looking for out of it. I do feel like I'm adding value by doing code review, but having me with sole control over the merge permissions &c. is seeming progressively less sensible.

I'm open to changing the ownership/management structure of the project, to make it easier/faster for you to get it to where you want it to be. I'm not sure what the best change would be -- seems like creating an organization is the typical first step?

@bskinn bskinn closed this as completed Aug 26, 2019
@jayvdb
Copy link
Contributor

jayvdb commented Aug 26, 2019

It seems I am not a collaborator any more either, as the ability to label & assign has also disappeared. It isnt that limiting anyway; not worth creating an organisaton for. I'm not a fan of merging my own PRs anyway, and I tend to work more in other peoples personal repos than my own.

It is immensely valuable to be working with you on this. Rarely is there someone else interested in providing detailed code review of this type of thing. And you've obviously thought about many of these features before, so you are not coming in cold. I'm just throwing in a few more, and some implementations which will hopefully get the project to a stage that it can get adoption / critical mass.

@bskinn
Copy link
Owner Author

bskinn commented Aug 26, 2019

Hm, odd. I will note, the last I looked, it still showed your 'collaborator' invite as pending. I'll resend.

As long as the workflow is satisfactory on your end, I'm content to keep things as they are. I just don't want the limitations on how rapidly I'm able to respond/act, due to this being a side project, to lead to frustration on your end.

@jayvdb
Copy link
Contributor

jayvdb commented Aug 28, 2019

Ah, accepted now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Changes/additions to the project documentation maybe This may or may not be implemented needs tests Tests are absent or incomplete for this
Projects
None yet
Development

No branches or pull requests

2 participants