4
\$\begingroup\$

For my programming class, at the start of the semester before we learned about object oriented coding, I was supposed to make a small program that searches through a bunch of files, being graded on useability.

from os import walk
from time import time

SEPARATORS = (" ", "-", "_", ":", "|", "~")
REMOVABLES = (".", ",", ";", "(", ")", "[", "]", "{", "}", "!", "?", "'", '"', "<", ">", "/", "`")


def inputs() -> [str, int]:
    """Takes the constraints from the user for the search"""
    term = input("Please enter your search term: ").lower()
    # Asks for the number of results the the user would like to see, and makes sure it is an int
    while True:
        try:
            result_count = int(input("How many results would you like to find: "))
            break
        except ValueError:
            print("Please enter only a whole number")
    return term, result_count


def data_to_dictionary() -> dict:
    """Turns the raw data in the text files into a dict"""
    data = {}
    # Creates a list of all the file names in the specified folder,
    # then opens those files and put the name and data of those files in a dict
    for path, path_name, files in walk("Data"):
        for file_name in files:
            with open(".\\Data\\" + file_name) as file:
                file_data = file.read()
            data[file_name[:-4]] = file_data  # The [:-4] is to remove the .txt extension
    return data


def word_matcher(term, data: dict, results: list, result_count: int) -> list:
    """Main search loop"""
    # If there is only one term, look through it once
    if type(term) == str:
        for data_item in data:
            # If the term is in one of the titles, add to results
            if term in data_item.lower() and data_item not in results:
                results.append(data_item)
        if result_count > len(results):
            for data_item in data:
                # If the term is in content of a document, and the document has not already been added, add to results
                if term in data[data_item].lower() and data_item not in results:
                    results.append(data_item)
    elif type(term) == list:
        for each_term in term:
            for data_item in data:
                # If the term is in one of the titles, add to results
                if each_term in data_item.lower() and data_item not in results:
                    results.append(data_item)
            for data_item in data:
                # If the term is in content of a document, and the document has not already been added, add to results
                if each_term in data[data_item].lower() and data_item not in results:
                    results.append(data_item)
    # Should never run
    else:
        print("Something went very wrong, please restart.")
        while True:
            pass
    return results


def word_splitter(term: str):
    """Splits the term"""
    # Removes items that may clog the search
    for item in REMOVABLES:
        while item in term:
            term = term.replace(item, "")
    # If there are things that separates words in the term, separate them
    for item in SEPARATORS:
        if item in term:
            term = term.split(item)
    # If there is empty str items in the list, remove them
    if type(term) == list:
        for item in term:
            if item == "":
                term.remove(item)
    return term


def result_print(result_list: list, result_count: int, search_start_time: float):
    print("")  # All of these are for spacing alone
    # If no results are found, loop until user closes program
    if not result_list:
        print("No Results Found")
        while True:
            pass
    # Removes results, so that it is only the number of results the user specified
    result_list = result_list[0:result_count]
    # Prints out all the results one line at a time
    for item in enumerate(result_list):
        print(f"{item[0] + 1}: {item[1]}")
    print("")
    print(f"It took {round((time() - search_start_time), 1)} seconds to find these results.")
    print("")
    while True:
        try:
            result_choice = int(input("Which file would you like to open. Put the number by its name: "))
            print("")
            with open(f".\\Data\\{result_list[result_choice - 1]}.txt") as file:
                print(file.read())
                print("")
                # If the user does not want to look at another article, break our of the loop, allowing the function to end
                if (input("Would you like to open another article? (Y/N): ").lower()) == "n":
                    break
        # If the input provided by the user is not a number, or a possible number, repeat the code
        except (ValueError, IndexError):
            print("Please only enter a number that is listed next to the file names.")
    print("")


results = []
term, result_count = inputs()
data = data_to_dictionary()
search_start_time = time()
results = word_matcher(term, data, results, result_count)
# Only does secondary check if max results has not been found
if len(results) < result_count:
    term = word_splitter(term)
    new_results = word_matcher(term, data, results, result_count)
result_print(results, result_count, search_start_time)

This program searches through a bunch randomly chosen (and some not randomly chosen) Wikipedia articles that I hand copied the title and intro paragraph from.

This is my first time making something that feels like it should be made more efficient. I don't know any efficiency techniques, so any advice there would be great. If you do add anything in this vein that are important things that I know about, please link some documentation for it.

Also, this is my first time touching os and dictionaries so if my implementation of walk() and data might not be the best.

Finally is there any problems with the general layout of the code or any other small things, like places were I should follow general practices if I don't?

Full code + files hear

Last night right after I finished the code, I watched this video and now the codes purpose feels really silly. ¯ \ _ (ツ) _ / ¯

\$\endgroup\$
3
\$\begingroup\$

Naming

Function names usually read nicely as verbs, since they "do" stuff. So word_splitter() might be a good name for a class, some reusable thing that splits words, but as a function it reads more descriptively as split_word() or split_term() (or maybe tokenize_term()). Similarly, result_print() reads more smoothly as print_results(). It's a command, it's doing something. data_to_dictionary() is ok, but load_data() or load_index() are more descriptive.

Also, generally, lists don't need "list" in the name. Parameters like result_list: list, especially with type annotations, usually imply that they are a list if they are plural, like results: list.

Optimizations

  • In word_matcher(), you repeatedly call .lower() on the contents of each document. Since you're only printing titles, not document contents, you could convert each document's text to lowercase once during the initial loading, and then not need to do so during the query/ies. This is appealing because the document contents are almost certainly much longer than your search queries, so cutting out extra loops is nice.

Code

word_matcher()

  • Notice how the behaviour for when term is a string vs a list is identical, aside from the loop. If you make sure it's always a list, by starting the function with

    if isinstance(term, basestring):
        term = [term]
    

    then you only need to handle the list-looping case, and can remove the first if-branch.

    Alternatively, you could make word_matcher() only take in single terms, and move the looping into the calling function.

    Either way, it would be nice for both readability and future maintenance to remove that duplication.

  • Generally isinstance() is preferable over a direct comparison with type(), because it will play nicely with inheritance. This doesn't matter for this program, because you're not subclassing str or anything like that, but it's nice in general.


result_print()

  • for item in enumerate(result_list):
        print(f"{item[0] + 1}: {item[1]}")
    
    Colloquially, this would use unpacking:
    for index, result in enumerate(result_list):
        print(f"{index + 1}: {result}")
    

data_to_dictionary()

  • This is fine. If you have many thousands of files (or it takes more than a second or two to load), you could consider using a ThreadPoolExecutor to read the data files concurrently using multiple threads.

    # If no results are found, loop until user closes program
    if not result_list:
        print("No Results Found")
        while True:
            pass

Looping infinitely with while True: pass is just going to heat up the room by occupying a CPU. If there is nothing more to do, quit. The user will notice. Use sys.exit() in a quick script like this, or return from the function.


Considerations

Ranking / Relevance

This is a search tool. If the volume of indexed data grows, you may want to order the results by some kind of measure of relevance, so that users can find their stuff more effectively. Maybe a match in the title is worth 3x more than a match in the body. Maybe when the term contains multiple words, results matching all of the term words are scored higher (and shown before) results matching just one word.

Search is a big field with endless optimizations. If this is interesting, check out Relevant Search by Turnbull and Berryman (2016), or stuff about search engine indexing and natural language processing.

Indexing

Right now, a search needs you to iterate through your entire index. At the cost of increased upfront processing time, and memory usage, you could do some pre-processing, and construct some more efficient indexes for faster searches.

Your data dict is a mapping of document name → contents. You could make an inverted index of term → document name, effectively pre-computing the results of any search term. Here is a simple example, you might also want to store things like the number of times the term appears in the document, to weigh into rankings:

from collections import defaultdict
import itertools, typing

def make_inverted_index(data: typing.Dict[str, str]) -> typing.Dict[str, typing.Set[str]]:
    terms = defaultdict(set)

    for doc_name, contents in data.items():
        body = [contents.lower()]
        for sep in SEPARATORS:
            body = list(itertools.chain.from_iterable(s.split(sep) for s in body)
        for term in body:
            terms[term].add(doc_name)

    return terms

and then, during queries, you can easily check if the query term is a key in the inverted index dict, without scanning all the contents.

\$\endgroup\$
2
  • \$\begingroup\$ Can I not just use "str" instead of "basestring"? \$\endgroup\$
    – K00lman
    Feb 24 '20 at 19:18
  • \$\begingroup\$ Also, it currently has some light sorting, were if the term is in the title it comes first and then if the whole term is found before it is split it comes first. \$\endgroup\$
    – K00lman
    Feb 24 '20 at 19:26
2
\$\begingroup\$

Looks pretty good to me! I liked that you used type hints.

A few tips:

  • The library pathlib is very good for path manipulation. You'll still need os.walk in this case, but you can use it to get the full filepath, just the filename, the stem (filename without extension) or the suffix (extension alone). I use it a lot.
  • You could use re.sub to take care of the separators and the removables. It might take some time to learn about regular expressions (short: regex), but it's worth the effort.
  • It's good practice to use isinstance(object, type) instead of type(object) == type.
  • It's also good practice to put the code in the bottom under def(main) and call main() under if __name__ == "__main__".
\$\endgroup\$
1
  • 1
    \$\begingroup\$ OP used type hints incorrectly. Instead of -> [type1, type2] it should have been from typing import Tuple, Dict and -> Tuple[type1, type2] and Dict[keytype, valuetype] instead of -> dict. Type hinting is usually pretty boring grunt work but this should still be pointed out \$\endgroup\$
    – Luke
    Feb 21 '20 at 5:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.