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

DM-14728: Add asEigenArray/Matrix returning Eigen::Map #14

Merged
merged 1 commit into from Jun 18, 2018

Conversation

r-owen
Copy link
Contributor

@r-owen r-owen commented Jun 18, 2018

No description provided.

@r-owen r-owen force-pushed the tickets/DM-14728 branch 2 times, most recently from a5b32a4 to 7c800ec Compare June 18, 2018 14:55
Copy link
Member

@TallJimbo TallJimbo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this reminds me why I had done the pybind11 suffix fix via a patch here: I assumed LSST was probably the only consumer of ndarray that couldn't just upgrade to pybind11 2.2.x to fix this problem, and hence the only consumer that would want this rather unportable, hacky fix. I think that's probably the right reasoning, so I think I may do a quick revert of that commit in ndarray, add a note to the README about pybind11 versions, push out 1.5.1, and then have you restore this patch.

@TallJimbo
Copy link
Member

ndarray 1.5.1 is out, without the pybind11 2.1.x suffix fix.

@r-owen
Copy link
Contributor Author

r-owen commented Jun 18, 2018

Can you update the CHANGELOG to say SWIG support is removed as well?

@TallJimbo
Copy link
Member

Can you update the CHANGELOG to say SWIG support is removed as well?

Already done.

Include free functions `asEigen`, `asEigenArray` and `asEigenMatrix`.
This version of ndarray also allows building without EigenView,
for compatibility with Eigen 3.3. But switching to that is left
for another ticket, as it will require some changes to our pybind11
wrappers.
@r-owen
Copy link
Contributor Author

r-owen commented Jun 18, 2018

I have updated to 1.5.1 and restored the patch. It builds on my Mac and I'm about to start another Jenkins run. Please have another look.

For future reference: please consider removing the .gitignore file when making the tarball; due to a bug in lsstsw rebuild it ends up replacing the original.

@TallJimbo
Copy link
Member

TallJimbo commented Jun 18, 2018

For future reference: please consider removing the .gitignore file when making the tarball; due to a bug in lsstsw rebuild it ends up replacing the original.

I just use GitHub's "draft new release" to make tarballs, which is way easier than doing them manually.

@r-owen r-owen merged commit 17ad4fa into master Jun 18, 2018
@ktlim ktlim deleted the tickets/DM-14728 branch August 25, 2018 05:50
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

Successfully merging this pull request may close these issues.

None yet

2 participants