jonwright / pyopengltk

OpenGL frame for Python/Tkinter via ctypes and pyopengl
MIT License
52 stars 14 forks source link

Convert into a proper package #9

Closed einarf closed 4 years ago

einarf commented 4 years ago

A suggestion splitting the project into a proper package. I think this makes the project a lot more pleasant to work with.

This should not change any behavior compared to the single module because everything that was in __all__ is imported in __init__.py. When there is no supported platform we get the same ImportError as before. setup.py is also change to include the new package instead of a single module.

demo.py and shader_example.py works as before in windows. I have not gotten the chance to test things in linux.

jonwright commented 4 years ago

This makes me a little nervous, I sometimes get confused about what happens during imports (don't run tests from your source folder, etc). The idea was to have a single file so you can also copy it into another project without adding a dependency. Organizing other things like examples and future docs into folders makes sense to tidy up, and if the code grows then we should come back to this. On the other hand, if this encourages people who are motivated to help then I don't want to block progress...?

einarf commented 4 years ago

This makes me a little nervous, I sometimes get confused about what happens during imports (don't run tests from your source folder, etc).

Right now the package/directory needs to be in your pythonpath. You can't drop it anywhere inside a project, but I could look into using local imports instead to make this possible? That should make the package as flexible as the original module.

Usually the location of the executed script will be added to the pythonpath (module/package discovery) or the current working directory will be used then running a module (python -m some.module)

If you have a project structure like this:

├── pyopengltk
│   ├── __init__.py
│   ├── base.py
│   ├── darwin.py
│   ├── linux.py
│   ├── opengl.py
│   └── win32.py
├── myproject/
│   ├── something.py
│   ├── stuff.py
├── entrypoint.py

Running it with:

python entrypoint.py

.. then the project root (location of entrypoint.py) will be added to the pythonpath and pyopengltk will in its current state will be found by imports.

If you copy the package/directory inside subdirectories in your project there will of course be import issues. Again, I can see if it's possible to use local imports across py2 and py3 to allow for this.

I'm more than willing to help out resolving any concern.

The idea was to have a single file so you can also copy it into another project without adding a dependency.

This is not uncommon. The only difference is that you copy a directory instead of a single module. Things should be exactly same as before unless you add new members to any of the submodules (you will have to import them in __init__.py).

(Except for the limitations pointed out in the above answer)

Organizing other things like examples and future docs into folders makes sense to tidy up, and if the code grows then we should come back to this. On the other hand, if this encourages people who are motivated to help then I don't want to block progress...?

I think organizing the package like this will encourage more development/contributors. The code is a lot easier to reason with and understand. It's also structured like the vast majority of python packages and is a lot easier to keep expanding.

einarf commented 4 years ago

Awesome 😄

Make sure you have fresh setuptools, wheel and twine before uploading and packaging is a breeze. 👍

pip install -U setuptools wheel twine

Do speak up if you have import issues when copying the package to a project. We can for example modify sys.path in __init__.py if the package is not in the root.