feat: add parameter to for output stream

[bpshaver]

The addition of the stream parameter allows the user to choose to send the yaspin spinner output to a stream or TextIO object other than sys.stdout. The most obvious use for this feature is to set the stream to stderr to support piping of output like python main.py > some.log and seeing the yaspin spinner in the terminal while stdout goes to some.log

[pavdmyt]

@bpshaver please run make fmt on your changes so that check-fmt check can succeed in CI.

[bpshaver]

@bpshaver please run make fmt on your changes so that check-fmt check can succeed in CI.

Done.

Does this seem like a sensible change? If so, I'll look through the failing test cases now.

[pavdmyt]

@bpshaver thanks for your contribution! I'll try to review this thoroughly next week.

[pavdmyt]

Apparently it is possible to detect situations when different streams are not tty:

import os

if not os.isatty(sys.stdout.fileno()):
    print("The output stream is not a TTY.")
else:
    print("The output stream is a TTY.")

So, I would rather prefer this approach, aligning behavior with e.g. https://github.com/pavdmyt/yaspin/issues/31#issuecomment-1016861612 to move further here. It must be much more convenient than requiring developer to define custom streams upfront. Plus, if yaspin is capable to find out which streams it is attached to, there is no need to rewrite older code.

[bpshaver]

@pavdmyt I do not understand how this is more convenient. The idea of detecting whether the target stream is a TTY and changing behavior or issuing a warning is a good one. But whether that stream is always stdout or whether it should be configurable by the user is a separate question.

If anything, https://github.com/pavdmyt/yaspin/issues/31#issuecomment-1016861612 supports making sys.stderr the default stream, not sys.stdout. That seems like a good idea to me, but I am suggesting a less drastic change, which is simply that yaspin open up the possibility for a user to send the spinner to stderr. It is also more explicit and it is backwards compatible. I disagree with your comment:

It must be much more convenient than requiring developer to define custom streams upfront.

In fact, my proposed change keeps sys.stdout as the default value (when stream is None) so it does not require anything from a developer who wants to continue using yaspin as before. Again, the change is completely backwards compatible. And there is no need to define custom streams. stdout or stderr should both be compatible streams and neither are "custom."

Plus, if yaspin is capable to find out which streams it is attached to

Just because yaspin can determine that doesn't mean it knows what it should do with the corresponding stream. If it detects that stdout is not a TTY, the user may want the spinner to appear in stderr, as in the case I show below. In general, explicit is better than implicit (Zen of Python).

My proposed changes support a file like this:

import sys
from time import sleep

from yaspin import yaspin


with yaspin(stream=sys.stderr):
    for i in range(5):
        print(f"{i} - Log some data")
        sleep(1)

being piped to a log file like so:

$ python foo.py > foo.log
⠋

where the spinner shows during script execution and the log file ends up looking like this:

0 - Log some data
1 - Log some data
2 - Log some data
3 - Log some data
4 - Log some data

Without stream = sys.stderr you get all the spinner artifacts in foo.log. Am I missing something? How does yaspin in its current state support this case?

[pavdmyt]

I think, I need to elaborate a bit more about my point. The idea behind capability to automatically detect streams to which yaspin is attached to allows to separate:

• spinner's stream (escape and color codes, spinner frames) from the
• useful text (.text attribute, write() and possibly .ok() and .fail() methods)

So that spinner's stream is always ends up in stderr (backwards incompatible change) and the useful text in stdout. This way, you can make decisions which stream ends up where by simply using redirects and the default behavior stays (visually) the same, as both stderr and stdout are attached to the TTY by default. This would allow doing python foo.py > foo.log (from your example above) having the useful text inside foo.log, while spinner's stream got rendered in the terminal window (as stderr remained attached to TTY). And this doesn't require any code changes using stream argument or attribute.

To summarize, we want to have spinner's stream and useful text clearly separated so that user can decide what to do with these streams. Having such feature, I'm not sure about the value of having an additional stream argument (please correct me if I'm wrong or in case I'm missing something).

[bpshaver]

I understand now, thanks!

I would call the .text, etc. accompanying text. Under normal circumstances, it is right next to the spinner. You're assuming that when the output is redirected to a non-TTY stream, however, the accompanying text should be interpreted as useful text and go to that non-TTY stream. But I'm not so sure that assumption makes sense for all users.

In the issue I was trying to tackle ( https://github.com/mandiant/capa/issues/1812 ) the accompanying text should go to stderr (remaining next to the spinner graphic) while useful text going to the non-TTY stream is everything contained within the spinner context:

$ python foo.py > foo.log
⠋ Analyzing data...

while program output goes to foo.log.

I understand a sensible default may be to send accompanying text to the non-TTY stream, but this example I think illustrates a case where that is not desired.

[pavdmyt]

In the issue I was trying to tackle ( https://github.com/mandiant/capa/issues/1812 ) the accompanying text should go to stderr (remaining next to the spinner graphic) while useful text going to the non-TTY stream is everything contained within the spinner context

I'm not sure I've understood this correctly, "accompanying text should go to stderr while useful text going to the non-TTY stream" - isn't accompanying and useful text are the same entities? It is also possible to redirect stderr to the non-TTY stream.

[bpshaver]

accompanying text is my name for whatever text normally goes right next to the spinner. You called it useful text, which is begging the question with respect to where it should go in the corner cases we're discussing. Call them what you like, but I'm pointing to three different types of text (not two):

• The actual spinner - should probably never go to a non-TTY stream
• Some text to the right of the spinner (.text, etc.) - maybe should go to a non-TTY stream. Probably depends on the user.
• Normal text written as result of print calls and the like - should be redirected as normal, unaffected by being within a yaspin spinner context.

I just don't see the utility in sending the spinner and any text to the right of it (determined by .text attribute) to different streams. That seems overly complex and probably a surprise to most users. Users can call print or whatever they during the normal execution of their code within the spinner context to send text to whatever stream they like. There's no reason for yaspin to make that decision for them.

[odisseus]

What is the status of this pull request? How can I help getting it merged and released?

[bpshaver]

Seems like @pavdmyt disagreed with my reasoning and the discussion ended.