Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Directory reorganization needed #3

Closed
HazenBabcock opened this issue Jan 29, 2016 · 9 comments
Closed

Directory reorganization needed #3

HazenBabcock opened this issue Jan 29, 2016 · 9 comments

Comments

@HazenBabcock
Copy link
Member

To reduce name space pollution most things should be moved to a sub-directory of a new folder called "storm_analysis", so the new layout would be something like:

storm_analysis/3d_daostorm
storm_analysis/decon_storm

..

@HazenBabcock
Copy link
Member Author

We also need a better name for the 3d_daostorm directory to make it easier to import.

@Dschoni
Copy link

Dschoni commented Apr 29, 2016

Wouldn't it be appropriate, to reorganize the whole project in a python module-like structure with according init.py files as well?

@HazenBabcock
Copy link
Member Author

HazenBabcock commented Sep 10, 2016

I renamed everything basically as described above, and I moved the 3D-DAOSTORM project to the daostorm_3d directory so that it can be imported more easily.

Can you elaborate your thoughts regarding init.py files and a module like structure?

@Dschoni
Copy link

Dschoni commented Sep 13, 2016

init.py (or more exact init.py files) are used to
mark a directory as a module. At some point, I feel that
end users want to import packages rather than setting
folders as working directories. Furthermore, it would be
a step towards distributing the whole project as a package/module
together with setup.py files that handle the installation/compilation
of external c-modules on the fly.

Am 2016-09-10 03:32, schrieb Hazen Babcock:

I renamed everything basically as described above. And I moved the
3D-DAOSTORM project to the daostorm_3d directory.

Can you describe a bit more what you mean regarding init.py files?

You are receiving this because you commented.
Reply to this email directly, view it on GitHub [1], or mute the
thread [2].

Links:

[1]
#3 (comment)
[2]
https://github.com/notifications/unsubscribe-auth/AH3ba_fbUaIIffL02XCfPDCFsDk9xhYYks5qogiggaJpZM4HPNg7

@HazenBabcock
Copy link
Member Author

HazenBabcock commented Sep 24, 2016

I believe that all the relevant folders already have __init__.py files in them, so I guess I still don't understand your suggestion. You only need to add the root storm-analysis directory to your Python path and after that you can access pretty much anything in this project using import statements.

I'm unsure about the best way to distribute this project because only parts of it are library functions. Also, my experience is that setup.py always fails when you are using the MinGW compiler on Windows. I've never figured out how to pass the correct arguments to setup.py to get it to work.

@Dschoni
Copy link

Dschoni commented Sep 29, 2016

The relevant init.py files are empty.
In order to distribute a package, all relevant imports
used for a module are defined in these init files, so that
f.e. submodules are imported automatically. Furthermore, there
should be an setup.py file in the root directory, that takes care
of compilation on the client and installing the package.
This is standard deployment for python packages rather than distributing
.sh scripts and the need to add something to path.

Making stuff windows compatible is another big thing. There are some
issues compiling against different versions of the python runtime and
basically
you have three options.

-Using the GNU toolchain with mingw (which I also don't really like)
-Compiling with Visual Studio (Which forces consistent c95 programming)
-Distributing precompiled dynamic libraries (which is kind of a
compromise...)

I see the main problem being the Fortran calls to LAPACK, which makes
stuff a
lot harder to cross-compile against. But as scipy also links to those
libraries,
someone, somewhere should have solved that problem already.

Am 2016-09-24 03:43, schrieb Hazen Babcock:

I believe that all the relevant folders already have INIT.py files in
them, so I guess I still don't understand your suggestion. You only
need to add the root storm-analysis directory to your Python path and
after that you can access pretty much anything in this project using
import statements.

I'm unsure about the best way to distribute this project because only
parts of it are library functions. Also, my experience is that
setup.py always fails when you are using the MinGW compiler on
Windows. I've never figured out how to pass the correct arguments to
setup.py to get it to work.

You are receiving this because you commented.
Reply to this email directly, view it on GitHub [1], or mute the
thread [2].

Links:

[1]
#3 (comment)
[2]
https://github.com/notifications/unsubscribe-auth/AH3bayj2M9-SZ7lawfviagolYLwbxnmKks5qtIBHgaJpZM4HPNg7

@hadim
Copy link
Contributor

hadim commented Oct 27, 2016

Now that storm_analysis is a Python module you could add a setup.py file so it makes it easier for people to install. However they will still need to manually compile C extension which is a pain on Windows.

So in addition to the setup.py file you could also build a conda-forge recipe (https://conda-forge.github.io/) so it makes it possible to distribute binary packages of storm_analysis for every platform (Windows, Linux, OSX). No more compilation needed, just install it with conda install storm_analysis and you're done.

I can help setting up the build system if needed.

@hadim
Copy link
Contributor

hadim commented Oct 27, 2016

A PR to add setup.py is available here #12

@HazenBabcock
Copy link
Member Author

This is now in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants