Get data from nfo file with functions Python

Question

I am learning python and I have made a function to get text from a .nfo file, improving on this block of code:

        # Book Author
        _author = ["Author:", "Author.."]

        logger_nfo.info('Searching Author book...')
        with open(nfofile, "r+", encoding='utf-8') as file1:
            fileline1 = file1.readlines()
            for x in _author:  # <--- Loop through the list to check
                for line in fileline1:  # <--- Loop through each line
                    line = line.casefold()  # <--- Set line to lowercase
                    if x.casefold() in line:
                        logger_nfo.info('Line found with word: %s', x)
                        nfo_author = line

        if nfo_author == '':
            logger_nfo.warning('Author not found.')

I've made this function:

nfofile_link = "The Sentence.nfo"
search_for = ["Author:", "Author.."]


def search_nfo(nfofile_link, search_for):
    logger_nfo.info('Searching nfo')
    with open(nfofile_link, "r+", encoding='utf-8') as file1:
        fileline1 = file1.readlines()
        for x in search_for:  # <--- Loop through the list to check
            for line in fileline1:  # <--- Loop through each line
                line = line.casefold()  # <--- Set line to lowercase
                if x.casefold() in line:
                    logger_nfo.info('Line found with word: %s', x)
                    global nfo_author
                    nfo_author = line


search_nfo(nfofile_link, search_for)
print(nfo_author)

This works, but I'd like to improve it, how can I do that?

An example of the .nfo file's content:

General Information
===================
 Title:                  Agent in Berlin
 Author:                 Alex Gerlis
 Read By:                Duncan Galloway
 Copyright:              2021
 Audiobook Copyright:    2021
 Genre:                  Audiobook
 Publisher:              Canelo
 Series Name:            Wolf Pack Spies 
 Position in Series:     01
 Abridged:               No

Original Media Information
==========================
 Media:                  Downloaded
 Source:                 Audible
 Condition:              New

File Information
================
 Number of MP3s:         42
 Total Duration:         11:33:05
 Total MP3 Size:         319.23 MB
 Encoded At:             64 kbit/s 22050 Hz Mono
 ID3 Tags:               Set, v1.1, v2.3

First the encoding, i think the nfo. file is not always ut8(can i use others encodings?). Is it proper to use global variable? in a way maybe something to improve. — Rose
– Rose, Commented Dec 11, 2021 at 13:27

FMc · Accepted Answer · 2021-12-11 16:06:59Z

Embrace functions, not global variables. It's good that you put your code in a function. However, you are not taking full advantage of the power of functions because you use the function to modify a global variable. Except under very rare circumstances, that is not a good idea. Instead, functions should take arguments and returns values. Organize your programs around that basic model.

Push algorithmic details into easily testable functions. A function that requires a file to exist is more awkward to test and debug than a function that takes text and returns a value. A simple data-oriented function, for example, can be pasted into a Python REPL and experimented with. An automated test for such a function is easy to write because you only need to define a list/tuple of strings and pass them into the function. For such reasons, I would reduce search_nfo() to the bare minimum: open the file, read the lines, and do any needed logging. That's it. Delegate most of the work -- in particular, the algorithmic details that might contain bugs, that might require debugging, testing, or future modification -- to a separate function.

Don't lowercase more often than you need to. Currently you are lowercasing values inside of nested for-loops. That's not a huge problem if the volume of data is moderate or small. Nonetheless, it's a decent idea to practice good habits of editing your code to weed out unnecessary operations. Don't take that virtue to unreasonable extremes, but keep it in mind.

def main():
    nfo_author = search_nfo('foobar.nfo', ('Author:', 'Author..'))
    print(nfo_author)

def search_nfo(nfofile_link, search_for):
    with open(nfofile_link) as fh:
        nfo_author = search_nfo_lines(fh.readlines(), search_for)
        return nfo_author

def search_nfo_lines(lines, search_for):
    targets = tuple(s.casefold() for s in search_for)
    for line in lines:
        lower_line = line.casefold()
        if any(t in lower_line for t in targets):
            # Do you want to return the original line
            # or the lowercase line? Adjust as needed.
            return line
    return None

if __name__ == '__main__':
    main()

To complete this question I would add what @n3buchadnezzar did: instead of loadig a whole file into memory file.readlines(), processing it line by line using a Generator by yielding. It also separates the concerns of file reading and content processing. — Miguel Alorda
– Miguel Alorda, Commented Dec 11, 2021 at 16:58
@MiguelAlorda Your point is often a good one, but it depends on the larger context. If the typical data file is like the example we were given (small to moderate in size), I would not bother with that unless some other aspect of the code base pushed me in that direction. Separation of concerns has already been accomplished by detaching opening/reading from parsing, as done here. Whether to switch from slurping-whole-file vs line-based-yielding is a different matter. — FMc
– FMc, Commented Dec 11, 2021 at 17:57
While @N3buchadnezzar solution is great for same type of .nfo, i feel this solution will be better for my problem. — Rose
– Rose, Commented Dec 11, 2021 at 18:51

N3buchadnezzar · Accepted Answer · 2021-12-11 21:20:07Z

I agree wholeheartedly with the other answers here, and I will just chip in my two cents.

Conversion of data structures You are reading in a nfo file, it would be wise to convert this into an object Python is more familiar with. If you are doing more with the file a NFO class is probably the best, but for now I feel the content of the file can best be expressed through a dictionary.

Returning from functions As mentioned previously we want to make full use of our functions and avoid global variables by returning or yielding. If we want the contents of the nfo file line by line we can do the following

from pathlib import Path
from pprint import pprint
from typing import Iterable


def path_2_nfo(path: Path) -> Iterable[str]:
    with open(path, "r+") as f:
        for line in f:
            if stripped := line.strip():
                yield stripped

The code above will return the contents of the nfo file line by line. You can for instance try to run the code below to see what it does

BOOK_PATH = Path(r"The Sentence.nfo")
nfo = path_2_nfo(BOOK_PATH)
for line in nfo:
    print(line)

I will also strongly recommend avoiding special characters in file names, but that is a story for another day. The second part is simply reading our nfo into a dict. From a first glance your nfo consists of three different objects: The header single line, contains no :, linebreak a full line of =, and lastly the content a line containing :.

Reading the file line by line and filtering based on these three types can look like

def nfo_2_dict(nfo: Iterable[str], sep: str = ":") -> dict:
    nfo_dict = dict()
    for line in nfo:

        is_linebreak = all("=" == char for char in line)
        is_content = sep in line
        is_header = not is_linebreak and not is_content

        if is_header:
            header = line
            nfo_dict[header] = dict()
        elif is_content:
            value, *content = line.split(sep)
            nfo_dict[header][value] = sep.join(c.strip() for c in content)


    return nfo_dict

Where our main now simply becomes

BOOK_PATH = Path(r"The Sentence.nfo")
nfo = path_2_nfo(BOOK_PATH)
nfo_dict = nfo_2_dict(nfo)

print(nfo_dict["General Information"]["Author"])
pprint(nfo_dict)

Depending on how often you need to search the file, I feel it is much cleaner to simply create a dictionary than searching the file each time.

If you used for line in f: instead of for line in f.readlines(): it would not load the whole file into memory before yielding it line by line. The thing returned by open is also a generator. And instead of Union[Iterator[str], list[str]] you could just use the easier to read and more generic Iterable[str]. — Graipher
– Graipher, Commented Dec 11, 2021 at 20:55
@Graipher great comments. i've updated my answer accordingly! I tried with just Iterator[str] and it complained when I added a list, but Iterable does the trick. Thanks — N3buchadnezzar
– N3buchadnezzar, Commented Dec 11, 2021 at 21:18

Stack Exchange Network

Get data from nfo file with functions Python

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Get data from nfo file with functions Python

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions