I’m talking about stuff like this: [file.unlink() for file in files] instead of the more verbose but maybe easier to grasp for python noobs:

for file in files:
    file.unlink()

Maybe with a bit more context:

def _cleanup(self) -> None:                                                                                                                                                                                                             
    dirs, files = partition(lambda f: f.is_file(), self._tmp_dir.rglob("*"))                                                                                                                                                            
    [file.unlink() for file in files]                                                                                                                                                                                                   
    [dir.rmdir() for dir in dirs]                                                                                                                                                                                                       
    self._tmp_dir.rmdir()
  • psukys@feddit.de
    link
    fedilink
    arrow-up
    11
    ·
    1 year ago

    Typically I would expect for list comprehension to be used for data generation or filtering, so something like this definitely strikes odd when reading code and would just ask to unpack into classic loop

  • nieceandtows
    link
    fedilink
    arrow-up
    3
    ·
    1 year ago

    I don’t think this is a good place to use list comprehension. Yes the code is smaller, but it is very hacky. It’s like those flight booking hacks where you book for a different destination with your actual destination as a transit location. Yes it’s cheaper, but that’s not an elegant solution.

  • slurp
    link
    fedilink
    arrow-up
    2
    arrow-down
    1
    ·
    edit-2
    1 year ago

    For me, the only reason I wouldn’t do this is to include debug level logging of which files are being removed.

    That said, how about using tempfile’s TemporaryDirectory to handle the clearing up of the temporary directory?

    • slurp
      link
      fedilink
      arrow-up
      1
      ·
      1 year ago

      Though you might want to consider whether you want it to remove as much as possible in the case of permissions errors, in which case you might want exception handling around each unlink call, with it raising an exception after it’s tried clearing everything if anything went wrong.