Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add python pbr based module packaging, adjust imports, fix tests #305

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@lukeplausin
Copy link

lukeplausin commented Mar 11, 2019

Hi Jaisen,

I tried to update commits from the old PR but just made it worse. So I just forked the repo again and committed the code again - problem solved.

Just to recap - here are the things which this PR would change:

CLA thing
Deleted my fork and recommitted all code as it was in the last PR (which I will close).

pip module
I made some changes on my fork to package Elodie up as a PIP module. This basically allows you to do some nice things using Pip:

You can install Elodie and all python dependencies using a one line command, and without needing to use git (this is probably a bit nicer for newbies):

pip install git+https://github.com/jmathai/elodie.git

The PBR packaging for pip includes CLI hooks, so you can invoke elodie as a CLI tool from any location:

cd (my pics folder)
elodie --source (my sd card) --destination .

You can install elodie in developer mode to the current environment - this allows you to make changes on the fly without reinstalling the module. You do still need git installed if you're installing via pip+git. If Elodie is accepted into the pypa public python repository then you wouldn't need git anymore.

cd (elodie github repo)
pip install -e .

gitignore:
These are standard includes for python. The python compiler will add files to the repo which will get checked into git unless they're excluded by .gitignore

python2 import
think I may have fallen for the double import trap. When importing submodules from within the same module you are supposed to use dot notation instead of writing the name of the module. It wasn't a problem in Python 2.7 until I moved elodie.py into the elodie module folder, and it was never a problem in Python 3 since the module loading system was changed significantly as of Python 3.3.

http://python-notes.curiousefficiency.org/en/latest/python_concepts/import_traps.html

I managed to run the nose tests locally on 2.7 and 3.7 so I'm confident the build should pass this time.

There is the future possibility to upload Elodie to the Python module index, so that it can be downloaded with pip in the same was as other public modules. Anyway, please let me know if you like it. If not I can change a few things around. I tested the electron app also and it seems to be working.

Best,
Luke

@coveralls

This comment has been minimized.

Copy link

coveralls commented Mar 11, 2019

Coverage Status

Coverage decreased (-1.3%) to 88.593% when pulling 7848fdb on lukeplausin:feature/python-module into 9ddb094 on jmathai:master.

@jmathai
Copy link
Owner

jmathai left a comment

Looks good. One small documentation suggestion and a symlink recommendation.

Otherwise it looks good! Thanks!

@@ -29,12 +29,10 @@ dnf install perl-Image-ExifTool

### Clone the Elodie repository

You can clone Elodie from GitHub. You'll need `git` installed ([instructions](https://git-scm.com/book/en/v2/Getting-Started-Installing-Git)).
You can clone Elodie from GitHub. You'll need `pip` and `git` installed ([instructions](https://git-scm.com/book/en/v2/Getting-Started-Installing-Git)).

This comment has been minimized.

@jmathai

jmathai Mar 13, 2019

Owner

Can we also add a link to install pip since that's a new dependency?

You can clone Elodie from GitHub. You'll need `pip` and `git` installed ([git instructions](https://git-scm.com/book/en/v2/Getting-Started-Installing-Git) and [pip instructions](https://pip.pypa.io/en/stable/installing/)).
@@ -11,23 +11,23 @@

This comment has been minimized.

@jmathai

jmathai Mar 13, 2019

Owner

What do you think about also symlinking it back to the original location? I imagine people have some automated scripts pointing to the current location which may fail and be difficult to troubleshoot if they simply do an update of the code.

Git seems to handle symlinks well enough. https://stackoverflow.com/a/954575

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.