hauntsaninja / pyp

Easily run Python at the shell! Magical, but never mysterious.
MIT License
1.41k stars 39 forks source link

if lines is used once, define it as a generator #23

Closed mgaitan closed 3 years ago

mgaitan commented 3 years ago

from #22

transparently optimise lines to a generator, if possible Currently lines is always a list. It's impossible to do this right in general, but one way that could work well is to optimise if we detect it's only mentioned once and that mention is either in a comprehension, loop or the second argument to map.

I implemented exactly that.

PS: pyp is great, thanks!

mgaitan commented 3 years ago

is lines being a list something that's actually caused an issue for you?

not really, although I'm still a very casual user. I've been a beginner with bash my entire life (happily, I guess) but I'm good enough with Python, so this looks pretty appropriate to calm my itches

So, I've just wanted to be helpful on this interesting project, nothing in particular.

cat README.md | pyp 'lines[4]'

I see. Well, I've just defined a wrapper that work as an indexable/sliceable iterator that also caches computed values. It seems to cover those cases

(pyp) tin@pop-os:~/lab/pyp$ cat lala.txt | pyp "[x.upper() for x in lines] + [x.title() for x in lines]"
LINEA 1
LINEA 2
LINEA 3
Linea 1
Linea 2
Linea 3
(pyp) tin@pop-os:~/lab/pyp$ cat lala.txt | pyp "lines[:2]"
linea 1
linea 2
(pyp) tin@pop-os:~/lab/pyp$ cat lala.txt | pyp "lines[2]"
linea 3
(pyp) tin@pop-os:~/lab/pyp$ cat lala.txt 
linea 1
linea 2
linea 3

However, I don't know if it worth the code pollution for little gain on most cases.

hauntsaninja commented 3 years ago

Thanks, that's a neat solution!

Pros:

Cons:

Missing feature:

I think this could cross into "mysterious" behaviour that pyp in general tries to avoid. Ideally, if I add support for user defined magic variables, Indexable would be something that people could just define in their pyp config and that way things would never be mysterious / people could customise the logic further. (I guess even today you could add Indexable into your pyp config, but you'd have to always do pyp 'Indexable(stdin)[1]' instead of say pyp 'ilines[1]').

Anyway, I think for now I'm not going to merge. If I get this more often as a feature request, or if I never get around to adding user defined magic vars, I might reconsider. Thanks again for this PR!

mgaitan commented 3 years ago

@hauntsaninja I agree! let me know if I could help with something less controversial :D

hauntsaninja commented 3 years ago

I implemented the magic variable support in configs described in the roadmap issue, see https://github.com/hauntsaninja/pyp#pyp-lets-you-configure-your-own-magic

This means users should now be able to do:

λ export PYP_CONFIG_PATH=config
λ yes | pyp 'ilines[1]'        
y

where config contains the Indexable class you thought of:

λ cat config
import itertools

class Indexable:
    def __init__(self, it):
        self.it = iter(it)
        self._computed = []
        self._exhausted = False

    def __getitem__(self, index):
        try:
            max_idx = index.stop
        except AttributeError:
            max_idx = index
        n = max_idx - len(self._computed) + 1
        if n > 0:
            self._computed.extend(itertools.islice(self.it, n))
        return self._computed[index]

    def __iter__(self):
        if self._exhausted:
            for element in self._computed:
                yield element
        else:
            for element in self.it:
                self._computed.append(element)
                yield element
            self._exhausted = True

ilines = Indexable(z.rstrip('\n') for z in stdin)