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

Restructure to normal python package #25

Merged
merged 1 commit into from
Oct 9, 2019
Merged

Restructure to normal python package #25

merged 1 commit into from
Oct 9, 2019

Conversation

simonvanderveldt
Copy link
Member

@simonvanderveldt simonvanderveldt commented Oct 8, 2019

As prepwork for publishing to pypi and to make it clearer/simpler to install and use.

Also fixed a bunch of PEP8/linting issues.
I only fixed the simple/non-invasive ones though, will have a look at the rest later.

How to use after these changes (see readme for more instructions):

$ pip install --user .
Obtaining file:///home/simon/src/monome/druid
Collecting prompt-toolkit>=2.0.10 (from monome-druid==0.1)
  Using cached https://files.pythonhosted.org/packages/87/61/2dfea88583d5454e3a64f9308a686071d58d59a55db638268a6413e1eb6d/prompt_toolkit-2.0.10-py3-none-any.whl
Collecting pyserial>=3.4 (from monome-druid==0.1)
  Using cached https://files.pythonhosted.org/packages/0d/e4/2a744dd9e3be04a0c0907414e2a01a7c88bb3915cbe3c8cc06e209f59c30/pyserial-3.4-py2.py3-none-any.whl
Collecting wcwidth (from prompt-toolkit>=2.0.10->monome-druid==0.1)
  Using cached https://files.pythonhosted.org/packages/7e/9f/526a6947247599b084ee5232e4f9190a38f398d7300d866af3ab571a5bfe/wcwidth-0.1.7-py2.py3-none-any.whl
Collecting six>=1.9.0 (from prompt-toolkit>=2.0.10->monome-druid==0.1)
  Using cached https://files.pythonhosted.org/packages/73/fb/00a976f728d0d1fecfe898238ce23f502a721c0ac0ecfedb80e0d88c64e9/six-1.12.0-py2.py3-none-any.whl
Installing collected packages: wcwidth, six, prompt-toolkit, pyserial, monome-druid
  Running setup.py develop for monome-druid
Successfully installed monome-druid prompt-toolkit-2.0.10 pyserial-3.4 six-1.12.0 wcwidth-0.1.7
$ druid

/cc @csboling

Partially fixes #11

Copy link
Collaborator

@trentgill trentgill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks fantastic!

I wonder if it's easy / possible / worthwhile to add druid-upload and druid-download to the PATH as well? That said, I wonder if they are necessary at all. Does the ease of not needing to type u <filename> or p inside of druid surpass the potential confusion of having the separate python programs? I'm leaning toward just-one-tool-that-does-those-things.

CC @csboling before I merge these changes, wanted to check if you have opinions about this direction?

I'm super excited to just be able to type druid in terminal!!

@csboling
Copy link
Contributor

csboling commented Oct 8, 2019

Oh man, awesome. Clearly your pipcraft is stronger than mine. I've tried to get this kind of thing working for scripts many times in the past and have always had some variation I wasn't really happy with. I'm also super in favor of the namespace reorg to make local imports work as expected.

@simonvanderveldt
Copy link
Member Author

simonvanderveldt commented Oct 8, 2019

Noticed we need to update the scripting tutorial if we merge these changes. Since the docs live in a different repo I'll create a PR there as well. Will do so tomorrow.

@trentgill
Copy link
Collaborator

I think we should also tag this release so we can tell people to download a certain version. another command-line arg would be good to query: eg: druid --v

@simonvanderveldt
Copy link
Member Author

Yeah, good point. Shall we tag a first release when we've rolled the upload and download actions into druid (assuming everyone agrees with that approach)?
We can add a -v to report version to that release as well.

@trentgill
Copy link
Collaborator

sounds great to me. do you want me to have a go at the upload/download, or do you want to jump in (i won't be able to get to it til tomorrow earliest)

@simonvanderveldt
Copy link
Member Author

simonvanderveldt commented Oct 8, 2019

I'll do it tomorrow, better to keep you free for other stuff :)

@simonvanderveldt simonvanderveldt force-pushed the python-package branch 2 times, most recently from 8eaf6b5 to cc27ce0 Compare October 8, 2019 22:48
@@ -0,0 +1,35 @@
#!/usr/bin/env python3

from setuptools import setup, find_namespace_packages
Copy link
Contributor

@csboling csboling Oct 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@simonvanderveldt I am getting an ImportError for find_namespace_packages on this line on Python 3.6.5 on an OSX VM. Not sure if this is maybe new in Python 3.7? Or maybe updating pip would fix it.

Copy link
Member Author

@simonvanderveldt simonvanderveldt Oct 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohw, yeah, that's annoying, you need a newer setuptools. They should just include it in the Python package but alas.
So that either means adding more instructions or assuming people have setuptools installed and switching to using find_packages and including __init__.py files.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the readme to include updating setuptools and I've removed the virtualenv section, users should just use pip install --user, people that want to develop/work on it I assume know what they're doing and even if we want to document it for them it should be in a separate readme.

@simonvanderveldt
Copy link
Member Author

I've added a PR to update the docs as well monome/docs#142, I think this is all ready.

@simonvanderveldt simonvanderveldt force-pushed the python-package branch 2 times, most recently from 6eff777 to f999951 Compare October 9, 2019 17:15
@trentgill
Copy link
Collaborator

Merged! I'll add the changes to scripting.md now to cover bowery.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically publish releases to pypi
3 participants