Reorganize

[petebachant]

This pull request moves things around and cleans things up a little:

• airfoilprep is now a package.
• Tests have been moved into their own module, which installs with the package for testing versions that are installed from PyPI, for example.
• A top level test script has been added for running tests on the local version during development.
• The airfoilprep.py CLI script is now installed in the Python scripts directory.

[andrewning]

Hi Pete,

Thanks for your contribution! I like many of the changes, a few I'm a little less sure about.

It's a good idea to wrap the test suite in a function. I'm less sure about moving it out of a test directory. That works well for this code, but this is one of several codes in our WISDEM suite and we'd like to maintain consistency. Not saying it's not worth making the change across our codes, but we'd have to reevaluate that. One reason we kept them in a test directory is for automated discovery with nose. Also some of our other modules have multiple separate test scripts and it just kept things cleaner. But keeping them in one directory might be easier for development than using python setup.py develop or setting the PYTHONPATH within a text editor's environment.

The separation of the CLI to a separate script and renaming the rest to core.py seems unnecessary to me. I don't think it's a bad idea, but it does remove some of the simplicity of the script for those who only use it from the CLI. That requires users to install before running the script (or to move the script and its adjoining folder around together). We have some users who don't really use Python and they just use the script by itself and so it is kind of nice to have everything in one file that doesn't require installation.

Also on importing, we avoid using * imports in favor of being explicit.

[petebachant]

Hi Andrew,

I just removed the top level test script but kept the airfoilprep.test module as part of the package. This allows you to run nosetests in the project directory for development since the test module is still automatically detected. This also allows testing the installed version from anywhere (I added the syntax to do this in the readme).

I think using a separate script for the CLI is necessary if you want to make this a package (to install the test module, and any other separate modules that may arise). Editing the PYTHONPATH or copy/pasting almost takes more time than running python setup.py install, achieves basically the same thing, and is kind of "hacky" in my opinion. Furthermore, doing a "real" installation allows the system to keep track of the version installed, which is useful if you ever want to put AirfoilPrep.py on PyPI or Anaconda.org.

Moving everything to core.py and importing all was done to avoid having airfoilprep.airfoilprep in the namespace. Once again, this is part of converting into a package to install tests.

Anyway, I may be wrong about all this! Just trying to make this look a little more like the typical Python packages out there.

Cheers,

[andrewning]

Pete,

I agree that those changes would be necessary if a package was required. However, I don't think that is necessarily the case. It certainly is a fine approach, but packages are not required for distribution they are purely organizational aids. One can instead add modules directly (using py_modules) and this works perfectly fine with setup.py, PyPI, etc.. Obviously, packages are great, and we use them in our other large modules, but for super simple repository like this one with only one file and one test script (I don't think I was adding the test script in setup.py and I should have), I've seen typical Python distributions go both ways. Putting this simple script in a package works just fine, but feels a little forced to me. But that's just a preference I guess, either way is totally acceptable.

Regarding using setup.py install, I definitely agree that's the way to go. That's certainly how I would recommend using it. However, what I was describing does not prevent standard usage. Many of our users are used to Fortran binaries as that is how all of the past NWTC tools have been distributed. By keeping the CLI in the airfoilprep.py file, the script could function in the same manner. You just make the script executable and then you can call airfoilprep.py somefile as if it was a binary. They don't edit the PYTHONPATH or copy/paste, they just drop in somewhere in their path like they do with their other binaries so that it is available everywhere. I would not support this atypical usage pattern if it prevented standard installation with setup.py, but in this case both can be supported just fine. The other reason for supporting this format is that airfoilprep.py as currently designed is really meant be stand-alone. The methods are not robust enough to be unmonitored, but that's a side issue.

I do agree that there are benefits to adding the test file directly in the src directory, but we'd need to reevaluate all of our packages before making a change like that for consistency reasons.

Also, the import * lines would need to be changed to be explicit. That's been a norm we've adopted. See: http://stackoverflow.com/questions/2386714/why-is-import-bad

[dykesk]

Andrew, this has been closed but i wonder if there are some of the commits that Pete implemented that you want to incorporate? Pete - I see you closed the issue - thanks for your contribution; it would be nice to see if we can find some middle ground since it looks like there was some good content added. Thoughts everyone?