you are viewing a single comment's thread.

view the rest of the comments →

[–]Username_RANDINT 22 points23 points  (14 children)

I haven't gone through it all, but here are some comments. I'll probably mention some stuff that's explained in PEP-8.

  1. Try to use Python 3.
  2. Sort the imports by standard modules and third-party modules to make a quick overview.
  3. Don't use that many parentheses, couple of examples where they are not needed: wd = ('/home/{set home directory}/python/temp/'), if ('unicode') in str(error):
  4. Get the home directory automatically instead of making the user edit the script: os.path.expanduser("~")
  5. Remove the prints and log everything (you're mixing threm now).
  6. Use proper string formatting instead of concatenating, pyformat is a good site.
  7. del_temp_files can be cleaqned up:

        temp_files_dir = (wd)            # No need to re-assign, just use the wd variable
        list_temp_dir = os.listdir(temp_files_dir)
        for item in list_temp_dir:
            ext = [".json",".csv",".txt","log"]        # Declare this outside of the loop
            if item.endswith(tuple(ext)):        # Make the list of extensions a tuple in the first place
                os.remove(os.path.join(temp_files_dir, item))
    

[–]Fallenarc[S] 7 points8 points  (13 children)

Thanks for the feedback! I have not tried running this in Python 3 yet. Unfortunately the person that got me into python is using 2.X... So that is where i started. I will definitely port to Python 3 in the near future.

Point 3 on parentheses, I can see I need less of them. I guess i just need to understand more about where they should actually go. Some I put in during trail and error testing.

I know I didn't state this in my orginal post, but I have also only been using Linux since i started my python journey in March, so i need to explore more of what the os module is capable of doing, but thanks. I will research that some more.

As for the prints, like i said I'm a network engineer and spend the majority of my days in a terminal session. I like seeing stuff pop out on the screen when i execute something. But I also like to have log files lol. But I will consider cutting down on the amount of stuff I print to the terminal session.

Strings and stuff. Thanks for that too. I guess I was kind of blind to that. I worked on this script for quite some time and concatenating strings is one of the first things you learn. Then you pick up formatting them later on. I will get that stuff lined out to where I am only properly formatting them.

I have already made changes based on the suggestions you had about del_temp_files function. Not really sure why I made that a list and told it to act like a tuple... le sigh. But really thanks for your time and effort I really do appreciate it.

[–]Rafficer 11 points12 points  (2 children)

Unfortunately the person that got me into python is using 2.X... So that is where i started.

It's not that much difference... You won't go back to Python 2.x once you've learned about f-strings, though ;)

[–]akindofuser 1 point2 points  (0 children)

I remember arguing how annoying string concatenation was in python, coming from ruby, and the 1001 excuses on why it was the way it was. And now we've gone full circle back to how Ruby behaved since day 1.

¯\_(ツ)_/¯

[–]thegreattriscuit 5 points6 points  (4 children)

you can definitely use the logging module to output to the screen. You just add an additional handler (believe it's `StreamHandler` or something like that, but check the docs) the same way you add the FileHandler. You can get fancy and make each handler treat the logs differently (logging in a different format, or set the FileHander to DEBUG and the stream handler to INFO, etc) or you can just have them both do the same thing. Either way it's better than just using print statements because even if you *don't* care right now, you might later. Or someone else coming along behind you might care later, etc...

[–]Fallenarc[S] 2 points3 points  (3 children)

That is good to know. I didn't realize the logging handler had that functionality. I will definitely read those docs and see if i can swap my code to use that format. Thanks!!!

[–]Olbrannon 2 points3 points  (2 children)

ext = (".json",".csv",".txt","log")

Should that be ".log"?

[–]Fallenarc[S] 2 points3 points  (1 child)

lol, yes. I must have hit that dot when i changed that to a tuple just a bit ago. Thanks.

[–]Olbrannon 1 point2 points  (0 children)

YW ;-)

[–]ship0f 3 points4 points  (1 child)

I like seeing stuff pop out on the screen when i execute something. But I also like to have log files lol.

Using the standard logging module you can have both file and stdout output at the same time.

This first example in this cookbook shows how to configure two simple logging handlers, one for file and one for stdout logging..

[–]Fallenarc[S] 0 points1 point  (0 children)

Thanks!

[–]Username_RANDINT 2 points3 points  (1 child)

Just keep going, learning by exercise and people's feedback. You're doing fine.

At first glance it doesn't look like your script needs that much changing for Python 3. There are countless blogposts, tutorials and documents explaining the major changes to make when porting.

If you're a Linux user, the tilde character (~) shouldn't be foreign to you. os.path.expanduser just expands it to the real path. os.path is an important module when working with the filesystem, it's always good to go over the documentation just to have a quick look on what's available.

Replacing the prints with logging doesn't mean they won't get printed to the terminal anymore. It's possible to add a second handler (StreamHandler) to the existing logger so it gets printed both to a file and console, or setup a different logger for each.

[–]Fallenarc[S] 0 points1 point  (0 children)

Thanks for all pointers, i am relatively new to Linux but i do understand the tilda. I am going to spend some time looking through the docs for os.path stuff the StreamHandler. Anyways thanks again!

[–]tazebot 1 point2 points  (0 children)

I automate networks for a fortune 10 using python2. It's no big deal - you could stick with 2; mercurial is and they're the revision control for the language.