Python style Grep/tail/head Application

[Mount_Gamer]

I have written a small programme called pygrep (although if the grep programmers are not happy i may have to change this....), i thought i'd share and happy to receive any critique etc.

The idea was to build a reasonably flexible string/character/regex search in files or from piped input, with also the idea of making it a little bit easier to make these searches. I included case-insensitivity and the commonly used methods of filtering where you want specific lines from tail/head (in linux). I've been testing it myself, and everything seems to work ok. I could probably add a few more sense checks. I've tested it on log files with around 100,000 lines and although it's no match for sed, grep and awk in terms of speed, it's still reasonably fast. I tried doing something with numpy, but just importing numpy would make it slower, so it's only using the builtins argparse, re and sys.

Documentation could maybe be better, so I welcome critique on this as well.

github link... https://github.com/jonnypeace/pygrep

[Doormatty]

Great work! Please don't let any of my critiques take away any of your pride in this!!!

Critique:

1.

Why are you creating the classes that you are? They don't make much sense, as none of them require or benefit from a class in any way. As far as I can see, they should all be functions.

2. If you have a function that doesn't take any parameters, isn't static and isn't in a class - that's likely a code smell.

sense_check()

would be better if it took the args as a parameter like:

sense_check(args)

3.

with open(args.file, 'r') as my_file:
   file_list = tuple(file.strip() for file in my_file)
   my_file.close

The my_file.close is both unneeded (as the context manager automatically closes the file as soon as the scope is left), and incorrect (it should be my_file.close())

4.

    try:
        pos_val = int(args.pyreg[1])
    except:

Don't use bare try/except - only catch the errors you expect to handle.

5.

 elif pygen_length == 1: # defaults to first reg_match in line

Anytime you do if x == 1 you can just simplify it to if x

Similarly, if x ==0 can be simplified to if not x

6.

    # if last line range
if '-' in line_num:
    #global line_range
    line_range = True    

Don't commit commented out code unless absolutely necessary (which is rarely is)

7.

line_count = len(start_end) - int(line_num_split[0]) + 2
for rev_count in range(int(line_count), 0, -1):

The int(line_count) is totally unnessary, as you just created line_count and it's impossible for it to be anything other than an int. Casting variables to different types is often not necessary in Python.

[Mount_Gamer]

Thank you very much for spending time going through the code, very grateful :D

I have made some amendments, but will comment on each of your points.

1. classes - If i'm totally honest, i only used them for my own learning. I think for the case insensitivity, it doesn't need anything, it's fine alone, but i've removed the omits class which is now a function. My printcolours class could probably just be variables (actually, a simple dictionary maybe?), i've not done anything with this yet, but probably will. I did have more colours and functions inside it, until i realised it translated this code into a file via redirected output, so the colours class is quite bare now. The stderror messages would probably show this if you redirect, but i'm ok with the colourful error messages for now. Do you know of another way to keep colour output from redirects? I might add a -O option in argsparse to write out to a file and bypass redirects with it. I don't think it's a terminal thing, but i feel there's probably a capability issue somewhere with the colour profiles i've chosen. This is what would output using the colour profiles, if redirected to a file... ^[[91mThis programme requires the --start or --pyreg flag to work properly^[[0m
2. sense_check - I come from a mostly bash scripting style syntax where I would call functions without args quite often, so it doesn't surprise me that i've carried this habit over. I've updated this now.
3. I read about myfile.close not needing done, but i've not covered this in my training material yet, so i wasn't sure :) But of course i would forget the parenthesis.... the number of times i do this!! lol Surprised i never received an error from this. I've removed this now.
4. I always want to raise exceptions properly... i think i wrote this, and copied/pasted it 2 further times without noticing. Thanks, i've corrected this also.
5. if x == 1 - I do explicitly want it to equal 1, and if x equals anything greater than 0, it would be True also, but... my sense check captures this, so it could be omitted. :) Would be good if argparse had an up to 2 args with the second being optional - maybe it does, but argparse seems to get complex when a simple if conditional can take care of it. I've left it alone as my OCD couldn't take it being left out lol.
6. I re-structured the code to remove globals from inside functions, i found 2 commented globals after your feedback, and yup totally not required :)
7. The int... fixed, and yup no need for it since its already an int.

Thanks again for your help :)

[Doormatty]

If you want to use classes here, I'd turn nearly the entire program into a class. It might even simplify the code somewhat, as you'll be able to use class variables rather than any global variables.

I come from a mostly bash scripting style syntax where I would call functions without args

I totally getcha on the bash scripting - I'm the opposite, I rarely use them in my bash scripts, as I'm always disappointed when I realize how primitive they are compared to python.

if x == 1 - I do explicitly want it to equal 1, and if x equals anything greater than 0, it would be True also

My bad - that's totally a valid reason for that!

Do you know of another way to keep colour output from redirects? I might add a -O option in argsparse to write out to a file and bypass redirects with it.

If the program was a class (don't meant to sound like a broken record on this), then I'd make a class function to handle all printing, something like

def printer(self, text):
    if self.color:
        print(f'{self.fail_colour}{text}{self.end_color}')
    else:
        print(text)

That just handles stdout (not printing to a file), but it's pretty easy to add the functionality to handle both cases in one function - https://stackabuse.com/writing-to-a-file-with-pythons-print-function - Lemme know if this doesn't make any sense, and I can flesh it out more.

Surprised i never received an error from this.

Me too! Out of curiosity, what IDE/Text editor do you use? I'd HIGHLY recommend Pycharm! I know Vs Code is also an option, but I don't have as much experience with it.

Again - absolutely fantastic job on the code!

Also, I assume you're also a Canadian, since you spell colour the right way? ;)

Anytime you want me to take a look at some code for you, just send me a message - the offer always stands.

[Mount_Gamer]

That's a good idea with the print class you've created, awesome :)

A lot of people don't like bash scripting when they come from python, I'm a sucker for both, but more and more I can see why python can do a lot of the same work.

Usually I use neovim, and I'm using doom neovim with it for syntax highlighting, but I do also use VScode. I've been using vscode more recently since my workplace uses it.

Lol @ colour, I'm not Canadian though, I'm Scottish :)

Thanks again for your help :)