cuspaceflight / firefish

CFD simulation software for Martlet 3
Apache License 2.0
5 stars 4 forks source link

Convert atmosphere model to use scikit-aero? #5

Open rjw57 opened 8 years ago

rjw57 commented 8 years ago

The fin flutter example includes a basic atmosphere model. There are more involved ones which are implemented by scikit-aero. It'd be a fun project for someone to convert finflutter.py over to using scikit-aero.

boyuanxiao commented 8 years ago

To run tests, I've added "scikit-aero" to the list of requirements in test/requirements.txt, however the automated tests installs this (old) package: https://pypi.python.org/pypi/scikit-aero. The newer package is https://github.com/Pybonacci/scikit-aero, and is documented as being accurate up to 86km instead of 11km. Do you know how can I change requirements.txt to install the newer version from Github?

rjw57 commented 8 years ago

We don't currently have a requirements file for the package as a whole. The interaction between requirements.txt and setup.py is subtle. (See: http://python-packaging-user-guide.readthedocs.org/en/latest/requirements/)

If scikit-aero has been untouched for 3 years and never released beyond 0.1, perhaps a better package might be found? I saw scikit-aero on the list of scikits and went no further before opening this issue. A quick Googling reveals https://pypi.python.org/pypi/AeroCalc/0.11 and http://pyaos.johnny-lin.com/?page_id=20 as potential sources for "better" packages.

That being said, the answer to your specific question is:

  1. Add "scikit-aero>0.1" to the install_requires argument in setup.py.
  2. Add "https://github.com/Pybonacci/scikit-aero/tarball/master#egg=scikit-aero" to a requirements.txt in the same directory as setup.py. Now running pip install -r requirements.txt will install the GitHub scikit-aero and so a version of scikit-aero matching the abstract version requirement[1] in setup.py.
  3. Arrange for pip install -r requirements.txt to be run by tox before installing the package. (Read the tox docs.)

But if scikit-aero is effectively dead, finding a better package or sticking with what we have is probably best.

[1] https://caremad.io/2013/07/setup-vs-requirement/

rjw57 commented 8 years ago

I've just noticed you added scikit-aero to tests/requirements.txt. This isn't quite the right place to add a requirement for the package as a whole. Ideally I'd like people to be able to type pip install cusfsim and have things just work. Only setup.py is read for package requirements. The {test,doc}/requirements.txt files are intended for additional packages which are needed specially for the test suite and the documentation. E.g. sphinx is needed to build the documentation but is not required, in general, to actually use the software.

I'll add some comments to the files and open a PR for this since having this documented in a random GitHub comment is sub-optimal :).