srsudar / eg

Useful examples at the command line.
MIT License
1.8k stars 98 forks source link

python x.7 code will break py3.7 #75

Closed farisachugthai closed 6 years ago

farisachugthai commented 6 years ago

https://github.com/srsudar/eg/blob/f2968471010fccf9e066d3064903eb49675ff766/eg/color.py#L90

One of the color functions only checks if python version is < x.7. However python3.7 is now the default py version so this might start breaking systems with newer python executables.

    def _color_helper(s, text, pattern, repl):
        # < 2.7 didn't have the flags named argument.
        if sys.version_info[1] < 7:
            compiled_pattern = re.compile(pattern, re.MULTILINE)
            return re.sub(
                compiled_pattern,
                repl,
                text
            )
        else:
            return re.sub(
                pattern,
                repl,
                text,
                flags=re.MULTILINE
)

So couldn't we do if sys.version_info < (2,7):

And be more explicit?

srsudar commented 6 years ago

Yikes what a terrible way I chose to do that. Shame on me.

Great catch. How on earth did you find that? Do you already have 3.7?

I'll take care of it this weekend.

farisachugthai commented 6 years ago

Oddly enough I was just skimming the code because the tldr repo mentions you! I love my cheatsheets, and I was excited to see a system written in Python with some configurability. I'm still on py3.6 btw. If I have a slow day at work I'll send a pull request

farisachugthai commented 6 years ago

Forgot to answer a question. On termux I have 3.6 on Ubuntu I use Conda as a pkg manager so I have 3.7 3.6 3.5 a couple 2.7s hahahaha

srsudar commented 6 years ago

76 employs your suggested fix. I didn't realize you could compare tuples like that. I'm surprised there haven't been any complaints given that 3.7 was apparently released over a month ago. If anyone chimes in that this is affecting them I will release immediately, otherwise I'll release around the 5th of next month on my regular schedule.

This also tipped me off that switching to py.test means the tests don't run on <2.7.

farisachugthai commented 6 years ago

Yup!

dir(sys.version_info)

Returns _lt_ and __gt__so you get intuitive comparisons like that. Gotta love the way the standard library implements operator overloading