1

I'm newbie to programming and in Python as well.
I wrote a function that implements the unix's tail:

def tail(file):
    strin = open(file, 'r')
    lis = strin.readlines()
    lastline = lis[-1]
    return lastline
    strin.close()

But I think that it is not optimal in performance.
How can I improve?

Vishnu Upadhyay
  • 5,043
  • 1
  • 13
  • 24
phar1no
  • 31
  • 1
  • 2

3 Answers3

8

You can use this Recipe from Collections.deque

def tail(filename, n=10):
    'Return the last n lines of a file'
    return deque(open(filename), n)

Refer this :- https://docs.python.org/2/library/collections.html#deque-recipes

Vishnu Upadhyay
  • 5,043
  • 1
  • 13
  • 24
  • This is the only proper pythonic way of doing tail, and I have found over 100 answers all involving subprocess or looping through the whole file. This issue is loading a log file with thousands of lines first to later just loop the last 100 lines seems inefficient, – TheNano May 14 '21 at 17:41
0

There's no need to store all the lines since you only want the last one:

lis = strin.readlines()
lastline = lis[-1]

More efficient is as per the answers to Iterate through Python list and do something on last element :

for line in open(file, 'r'):
    pass
else:
    return line

(I was not aware of the Collections.deque solution and I agree it's better, also parameterizable for n lines)

Community
  • 1
  • 1
smci
  • 32,567
  • 20
  • 113
  • 146
  • 2
    What's the difference, you just copy-pasted two lines from question. Or am I missing something? – luk32 Oct 14 '14 at 08:16
  • 2
    I identified the wasteful part, which is what OP was asking for. That's what you missed. And I just edited in the better alternative. Not so fast with the downvote. Kindly reverse. – smci Oct 14 '14 at 08:18
  • 1
    No, the question is "How can I improve?". Not "what part is bad". It looked you tried to give a suggestion but only copied the same part. Also I don't think your solution is any improvement, because `readlines()` will load whole file into memory anyways, then iterate. So both solutions are essentially the same. You should use `readline()`, to process lines 1-by-1, and then you would have a better memory-constraint solution. – luk32 Oct 14 '14 at 08:26
  • 1
    @grc, luk32: so we obviate `readlines()` by directly iterating on `open(file, 'r')`. – smci Oct 14 '14 at 08:30
  • @luk32: I started out by highlighting the inefficient part, while researching and then 2 min later posting an improvement suggestion, citing previous similar question. There's no need to attack my reading comprehension: in this case "How can I improve?" specifically boils down to "what part is bad". – smci Oct 14 '14 at 08:33
  • Now, you do. Previously you didn't do much. I didn't cast any vote. Just commented on how to improve. I think this is nice for one last line. Though I like `deque` solution more, as it's more flexible when the time for more "last-lines" will come. – luk32 Oct 14 '14 at 08:34
  • 1
    Good man, I didn't attack your reading comprehension, just belittled your defense of an incomplete answer. It was not helpful at all IMO. "How can I improve", is not the same to "where is it suboptimal", with the difference being, proposing an improvment, which 1st revision failed to even attempt to do. – luk32 Oct 14 '14 at 08:39
  • @luk32: not true. I had already researched it and was on the point of posting it, 22 mins ago, as you can confirm from the log. So not so fast, whoever the downvoter was. The deque solution is much nicer and generalizes to n lines, I wasn't aware of that. – smci Oct 14 '14 at 08:42
0

There are a couple of problems with your code. First you are closing the file after the return. Everything after the return statement will not be reached. Second, when you're working with files, you should use with. It opens your file, and when it leaves the block it will close it for you. And at last, you can merge three lines to an oneliner.

I would wrote it like this:

def tail(file):
    with open(file, 'r') as o:
        return o.readlines()[-1]
fredtantini
  • 15,966
  • 8
  • 49
  • 55
Vincent Beltman
  • 2,064
  • 13
  • 27