search

strohel / Ceygen

Cython helper for linear algebra with typed memoryviews built atop of Eigen C++ library
MIT License
36 stars 5 forks source link

Upgrading installation to setuptools #4

Open Dapid opened 9 years ago

Dapid commented 9 years ago

This should close #3, and along the way, also #2.

I am having some trouble linking to Eigen in my box, here is the log: https://gist.github.com/Dapid/94cff5f6f3b4124a23da

The problem seems to be lu.

Dapid commented 9 years ago

Travis is using an older Cython that doesn't support explicit relative imports. Upgrading it and retriggering.

Dapid commented 9 years ago

Finally it works! I have bundled a bunch of things in this PR, but it was needed to make it work.

Travis is using Eigen 3.0.5, but the installation fails on Fedora with Eigen 3.2.5, as the log shows. I will go through the release notes and see if there has been any change.

strohel commented 9 years ago

Thanks for this pul request, I'll look at the commits more closely as soon as I find some time.

One thing to note now, however: would you mind changing the commit messages to be in simple (not continuous) present tense? i.e. "migrate to setuptools" instead of "migrating to setuptools". This is what git developers themselves use, see https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches?id=HEAD

Also, I would prefer if you squash the commit fixups, i.e. hide the sausage-making. (see also https://sethrobertson.github.io/GitBestPractices/#sausage )

Dapid commented 9 years ago

I have squashed it into three logical commits, that I think will make it easiest to review. All tests pass locally and on Travis.

strohel commented 9 years ago

Thanks, @Dapid - sorry for being extremely slow to respond, I've been rather busy lately, you know that.. I'll have a look at the latest version when time permits.

Dapid commented 9 years ago

Don't worry, I understand. Better to get this right.