USEPA / LCIAformatter

MIT License
24 stars 9 forks source link

minor installation issue on linux machine #59

Closed mfastudillo closed 3 years ago

mfastudillo commented 3 years ago

pip install git+https://github.com/USEPA/LCIAformatter.git@joss_review#egg=lciafmt throws an error when trying to install on a ubuntu machine. The problem seems to be that pyodbc would not install with pip install pyodbc ... see [pyodbc install instructions]. I therefore installed first unixodbc-dev and then pip install git+https://github.com/USEPA/LCIAformatter.git@joss_review#egg=lciafmt run apparently successfully. However if I try to import lciafmt from a ipython session I get the following error:

fatal: not a git repository (or any of the parent directories): .git

I am not sure how this problem can be handled, but I guess it can be at least noted on the installation instructions.

openjournals/joss-reviews#3392

tngTUDOR commented 3 years ago

My flow to install with Linux is as follows:

conda create -y -n lciafmt python=3.9
conda activate lciafmt
conda install pyodbc
pip install git+https://github.com/USEPA/LCIAformatter.git@joss_review#egg=lciafmt

And this install will work fine. However, pyodbc installed on linux like this will not work. See below for the error message.

>>> lciafmt.clear_cache()
INFO delete cache folder /tmp/lciafmt
>>> impact_m = lciafmt.get_method(lciafmt.Method.ImpactWorld)
INFO get method ImpactWorld+
WARNING Please install drivers to remotely connect to Access Database. Drivers only available on windows platform.For instructions visit: https://github.com/mkleehammer/pyodbc/wiki/Connecting-to-Microsoft-Access
INFO download from https://www.dropbox.com/sh/2sdgbqf08yn91bc/AABIGLlb_OwfNy6oMMDZNrm0a/IWplus_public_v1.3.accdb?dl=1 to /tmp/lciafmt/Impact_World.accdb
INFO read ImpactWorld+ from file /tmp/lciafmt/Impact_World.accdb
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/maynardjk/workspaces/joss/LCIAformatter/lciafmt/__init__.py", line 81, in get_method
    return impactworld.get(file=file, url=url)
  File "/home/maynardjk/workspaces/joss/LCIAformatter/lciafmt/iw.py", line 36, in get
    df = _read(f)
  File "/home/maynardjk/workspaces/joss/LCIAformatter/lciafmt/iw.py", line 61, in _read
    cnxn = pyodbc.connect(connStr)

pyodbc.Error: ('01000', "[01000] [unixODBC][Driver Manager]Can't open lib 'Microsoft Access Driver (*.mdb, *.accdb)' : file not found (0) (SQLDriverConnect)")

Here is the list of installed packages:

> pip freeze
appdirs==1.4.4
certifi==2021.5.30
chardet==4.0.0
esupy @ git+git://github.com/USEPA/esupy@732eca80b73baea1a0c68799628c22ccd7d54728
fedelemflowlist @ git+git://github.com/USEPA/Federal-LCA-Commons-Elementary-Flow-List@50a4df6dad7259b8d12f735367850fe48e83badd
idna==2.10
lciafmt @ git+https://github.com/USEPA/LCIAformatter.git@9ad06b8305763e24198f65b44a53c61544fc8767
numpy==1.21.0
olca-ipc==0.0.10
pandas==1.3.0
pyarrow==4.0.1
pyodbc==4.0.31
python-dateutil==2.8.1
pytz==2021.1
PyYAML==5.4.1
requests==2.25.1
requests-ftp==0.3.1
six==1.16.0
urllib3==1.26.6
xlrd==1.2.0
bl-young commented 3 years ago

@mfastudillo

fatal: not a git repository (or any of the parent directories): .git

This error occurs when you are not connected to a git - from this block. Despite it being described as a fatal error, it has limited impact except that the git-hash won't be appended to the file name of any method you produce. You should be able to proceed with all functions. It would be good to find a way to avoid that error, or at least explain it as such in the readme.

bl-young commented 3 years ago

@tngTUDOR

WARNING Please install drivers to remotely connect to Access Database. Drivers only available on windows platform. For instructions visit: https://github.com/mkleehammer/pyodbc/wiki/Connecting-to-Microsoft-Access

We have added this warning to point users to this explanation for how to download the correct driver for pyodbc if you don't have it installed. However it seems this may not be feasible in linux. For ImpactWorld+, it might be limited to Windows only.

WesIngwersen commented 3 years ago

What about making pyodbc an optional dependency in the setup.py, like reference in setup tools docs

setup(
...
extras_require={
        "ImpactWorld+":  ["pyodbc>=4.0.30;platform_system='Windows'"],
    }
}

As I interpret the docs, this setup will not install pyodbc by default with pip install. You would have to use

pip install LCIAformatter["ImpactWorld+"]

to install it, and still then it would only install pyodbc if it is a Windows machine due to the statement of that dependency afterwards.

WesIngwersen commented 3 years ago

Following that to also make pyodbc behave like an optional requirements in respect to requirements.txt we could move that line into an ImpactWorld_requirements.txt as pip permits install with multiple requirements files

WesIngwersen commented 3 years ago

@mfastudillo @tngTUDOR I have implemented the changes I suggested in my comments. Can one of you test that it works on Linux and will not install pyodbc? I hope this solution is acceptable to you given the constraints of the Impact World+ data being published as an Access db for this version. We can communicate with that team to note this limitation.

tngTUDOR commented 3 years ago

I agree @WesIngwersen, it would be better for Open Source Initiatives if ImpactW+ was published

You have all my support to ask the IW+ team about this two things. Maybe they can use lciaformatter with a windows machine to directly publish a json-ld version on github

mfastudillo commented 3 years ago

I tried

python3 -m venv eparev6
source eparev6/bin/activate
pip install git+https://github.com/USEPA/LCIAformatter.git@joss_review#egg=lciafmt

and it does install lciafmt without installing pyodbc (although it complains of wheel not being installed). But if I try to import lciafmt I run into the same problem.

image

Nonetheless I can import the methods e.g. lciafmt import get_method and they seem to work fine

WesIngwersen commented 3 years ago

@mfastudillo That error message is from trying to run a git command, so it seems import lciafmt is indirectly calling git. I'm guessing that is here when we try to get a git hash for the current module, but it's inside a try clause, so I'm not sure why its returning that error. This is probably a separate issue.

bl-young commented 3 years ago

The git hash issue in the most recent comment has been resolved (#60).

I believe this issue can also be closed with the updated install procedure for linux.