callat-qcd / espressodb

Science database interface using Django as the content manager.
https://espressodb.readthedocs.io
BSD 3-Clause "New" or "Revised" License
8 stars 3 forks source link

[JOSS review] Installation from source requires pre-installed Django #41

Closed ixjlyons closed 4 years ago

ixjlyons commented 4 years ago

Minor issue. The README currently lists the following steps to install EspressoDB from source for testing:

To run the tests, clone this repo, install the dependencies

pip install .
pip install -r requirements-dev.txt
pip install -r example/my_project/requirements.txt

The first step fails in a clean environment because espressodb/__init__.py imports Django. It may also be worth noting that installing the sdist from PyPI also fails for the same reason:

pip install --no-binary espressodb espressodb

I see the latter as pretty much a non-issue though since you're providing a universal wheel. I think the simplest solution would be to re-order the steps so the requirements are installed first. Perhaps there's a way to defer importing Django for settings -- I'm not sure.

https://github.com/openjournals/joss-reviews/issues/2007

ckoerber commented 4 years ago

Hello @ixjlyons,

Thank you for pointing out this issue.

The easiest fix would indeed be updating the docs to run the requirements install before the espressodb install.

I believe it should be possible to place the espressodb/__init__.py Django import in the provided init function. This would delay the import until init() is actually called and thus allows to run a no-binary install. I have to run some checks—if this works out and does not affect other functionalities, I will include the delayed import in the next version.

ckoerber commented 4 years ago

The commit 9df504e should resolve this issue (no doc update needed). This should also allow the no-binary install which will be tested once the new version is ready for shipping.

ckoerber commented 4 years ago

The no binary install now works with v1.1.0.

Thank you for your feedback @ixjlyons.