Skip to main content
deleted 2 characters in body
Source Link

A few suggestions:

  1. This doesn't need to be a one line function. Writing it on one line makes it much harder to understand and makes finding mistakes more difficult.
  2. Use os.path.join to assemble your full pathname + wildcard + extension. That way, you don't need to worry about whether or not your pathname is given with or without a trailing /
  3. If you're going to worry about a trailing / in your pathname, you should be equally worried about a preceding . in your extension.
  4. glob.glob already returns a list with the matching files. I don't see any need for the complicated, nested list comprehensions you're using. Your function should essentially be a wrapper for calling glob.glob that simply makes combining the pathname + extension a little easier for the user.

That being said, here's how I might re-write this function:

import glob
import os

from typing import List

def find_files_with_extension(path_name: str, extension: str) -> List[str]:
    extension_wildcard = "*"

    if extension[0] != ".": //# suggestion #3
        extension_wildcard += "."
    extension_wildcard += extension

    path_name_wildcard = os.path.join(path_name, "**", extension_wildcard) //# suggestion #2

    return glob.glob(path_name_wildcard, recursive=True)

A few suggestions:

  1. This doesn't need to be a one line function. Writing it on one line makes it much harder to understand and makes finding mistakes more difficult.
  2. Use os.path.join to assemble your full pathname + wildcard + extension. That way, you don't need to worry about whether or not your pathname is given with or without a trailing /
  3. If you're going to worry about a trailing / in your pathname, you should be equally worried about a preceding . in your extension.
  4. glob.glob already returns a list with the matching files. I don't see any need for the complicated, nested list comprehensions you're using. Your function should essentially be a wrapper for calling glob.glob that simply makes combining the pathname + extension a little easier for the user.

That being said, here's how I might re-write this function:

import glob
import os

from typing import List

def find_files_with_extension(path_name: str, extension: str) -> List[str]:
    extension_wildcard = "*"

    if extension[0] != ".": // suggestion #3
        extension_wildcard += "."
    extension_wildcard += extension

    path_name_wildcard = os.path.join(path_name, "**", extension_wildcard) // suggestion #2

    return glob.glob(path_name_wildcard, recursive=True)

A few suggestions:

  1. This doesn't need to be a one line function. Writing it on one line makes it much harder to understand and makes finding mistakes more difficult.
  2. Use os.path.join to assemble your full pathname + wildcard + extension. That way, you don't need to worry about whether or not your pathname is given with or without a trailing /
  3. If you're going to worry about a trailing / in your pathname, you should be equally worried about a preceding . in your extension.
  4. glob.glob already returns a list with the matching files. I don't see any need for the complicated, nested list comprehensions you're using. Your function should essentially be a wrapper for calling glob.glob that simply makes combining the pathname + extension a little easier for the user.

That being said, here's how I might re-write this function:

import glob
import os

from typing import List

def find_files_with_extension(path_name: str, extension: str) -> List[str]:
    extension_wildcard = "*"

    if extension[0] != ".": # suggestion #3
        extension_wildcard += "."
    extension_wildcard += extension

    path_name_wildcard = os.path.join(path_name, "**", extension_wildcard) # suggestion #2

    return glob.glob(path_name_wildcard, recursive=True)
Source Link

A few suggestions:

  1. This doesn't need to be a one line function. Writing it on one line makes it much harder to understand and makes finding mistakes more difficult.
  2. Use os.path.join to assemble your full pathname + wildcard + extension. That way, you don't need to worry about whether or not your pathname is given with or without a trailing /
  3. If you're going to worry about a trailing / in your pathname, you should be equally worried about a preceding . in your extension.
  4. glob.glob already returns a list with the matching files. I don't see any need for the complicated, nested list comprehensions you're using. Your function should essentially be a wrapper for calling glob.glob that simply makes combining the pathname + extension a little easier for the user.

That being said, here's how I might re-write this function:

import glob
import os

from typing import List

def find_files_with_extension(path_name: str, extension: str) -> List[str]:
    extension_wildcard = "*"

    if extension[0] != ".": // suggestion #3
        extension_wildcard += "."
    extension_wildcard += extension

    path_name_wildcard = os.path.join(path_name, "**", extension_wildcard) // suggestion #2

    return glob.glob(path_name_wildcard, recursive=True)