haasad / EcoInventDownLoader

Download, unpack and import ecoinvent into your brightway2 project in one simple step
MIT License
13 stars 6 forks source link

Appdirs issue on mac? #12

Closed pjamesjoyce closed 5 years ago

pjamesjoyce commented 5 years ago

Have you had any issues parsing the appdirs path on mac?

A colleague who uses a mac is trying to install lcopt but can't extract the 7z file when it's downloaded into ~/Library/Application Support/EcoInventDownLoader/EcoInventDownLoader

I think the space in Application Support is being escaped to Application\ Support and then parsed literally by python, which breaks the extract command

This also breaks various bits of bw2io and lcopt... basically whenever you need to open a file with a space in the path

Have you come across this, or could it be this particular mac?

haasad commented 5 years ago

I don't have access to a Mac right now, but brightway has used appdirs for a long time and I never had any issues on Mac. Therefore it's likely eidl doen't handle the path safely somewhere. I'll investigate and report back.

pjamesjoyce commented 5 years ago

Excellent, thanks! I don't have access to a Mac either... I figured brightway must work on Mac though, so it's a weird error

pjamesjoyce commented 5 years ago

Fyi I've gone with:


def fix_mac_path_escapes(string):
    return string.replace(r"\ ", " ")

For the bits in lcopt that break for time being to see if that fixes his problem

haasad commented 5 years ago

I think the pythonic way would be to use https://docs.python.org/3/library/pathlib.html instead of doing string manipulations, but I've never taken the time to look into the module. It's in the standard lib since python 3.4.

haasad commented 5 years ago

The problem was indeed caused by the space in the path, but not connected with escaping the space character. Problem was in these two line of codes:

extract_cmd = '7za x {} -o{}'.format(self.out_path, target_dir)
self.extraction_process = subprocess.Popen(extract_cmd.split())

subprocess.Popen accepts a sequence of program arguments as list. The code above simply uses str.split to split the command string into a list. This naïve approach fails when there is a whitespace in the path. Fixed code uses a list from the start: https://github.com/haasad/EcoInventDownLoader/blob/b6aa722262524f74e2bdc1e8353d1c99af047a24/eidl/core.py#L153-L154

It's fixed in #13 and I tested it on mac and linux. If you continue having problems with this in lcopt, please let me know @pjamesjoyce. In that case there must some additional problem with the path handling.

pjamesjoyce commented 5 years ago

Excellent! Thanks @haasad