jirassimok / sky-python-music-sheet-maker

Make visual music sheets for thatskygame (graphical representations of the Sky keyboard)
MIT License
1 stars 0 forks source link

Make this a proper Python package and command-line tool #1

Open jirassimok opened 4 years ago

jirassimok commented 4 years ago

I'm interested in turning this into a regular Python package (using setuptools), which would make it easier to install (e.g. using pip) and more convenient to use (e.g. running skymusic from any directory, rather than python main.py from the source directory). For the same reasons, I'd also like to add a few command line options, such as specifying the output directory.

I threw together a simple setup.py (see this branch), but I ran into some issues that prevent it from working. And without a few significant changes to the project, I don't think it can be made to work.

Package directory structure

Because the root package is in the python directory, setuptools only recognizes it as a package named "python." There is an option (package_dir) that is supposed to fix this, but other parts of the library don't work when using it.

The solution to this is to rename the python directory to something more suitable, or to move everything in it into a subdirectory (e.g. python/skymusic). The second option is likely best, as it allows other top-level packages to be added later without cluttering the project's base directory.

There is one workaround I thought of: during the execution of setup.py, it could copy the entire directory into one with a better name, and then use that one as the package source before deleting it.

Resource files

The other issue is that the project's resource files (and output directory) are all accessed using hard-coded paths relative to project or source code. This makes it harder to include the resources in the package with setuptools, and it makes it harder to run the program from another directory.

The best solution would be to use importlib.resources to manage resource access. This would require Python 3.7 or newer, or the backport importlib_resources.

The biggest issue with this might be that it requires the resources to be located in the Python source directory, which means they would all need to be moved.

Proposed changes

Basically, I'm proposing that the project use a more-standard Python directory structure to facilitate its use as a command-line tool.

The changes I'm proposing would result in a directory structure something like this:

/
├── .gitignore
│   ⋮
├── setup.py
└── src/
    └── skymusic/
        ├── __init__.py
        │   ⋮
        ├── instruments.py
        └── resources/
            ├── __init__.py
            ├── css/
            │   ├── __init__.py
            │   ├── main.css
            │   └── warning.css
            └── elements/
                ├── __init__.py
                ├── A-circle.png
                │   ⋮
                └── unhighlighted-chord.svg

With this, structure, you can access resources using importlib.resources. The easiest way to convert a lot of the current resource accesses might look something like this:

from importlib import resources
import io
from PIL import Image

Image.open(io.BytesIO(resources.read_binary(
    'skymusic.resources.elements', 'A-circle.png'))

(You could skip the use of BytesIO by using resources.open_binary, but it might cause memory leaks (but not significantly more than you get from not closing Images).)

Benefits

The main benefit of making the project into a package is that it could be installed (along with its dependencies) using pip and run as a shell command.

$ pip install git+https://github.com/sky-music/sky-python-music-sheet-maker.git
...
$ skymusic
===== VISUAL MUSIC SHEETS FOR SKY:CHILDREN OF THE LIGHT =====
...
tracey-le commented 4 years ago

I support changing the repository structure, (I appreciate notice on best practices and contributions like these a lot) yeah we will have to do it afterwards

jmmelko commented 4 years ago

Yes, but we’ll have to get the work in the dev branch done first. Indeed, we have decided to maintain only one code, which has to work on both Discord and stdout. This code is currently being written in the dev branch.

tracey-le commented 4 years ago

I'll assign myself the task of migrating our project to a more standard directory structure.

(This may take a while, amongst other things I have to do. And making sure everything still works lol, but I'll post updates)

jmmelko commented 4 years ago

Yes but Tracey, before doing that and the order thing, could you please pull dev to beta?