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

Replace usage of deprecated Eigen class MappedSparseMatrix. #3499

Merged
merged 4 commits into from
Nov 23, 2021

Conversation

rmlarsen
Copy link
Contributor

@rmlarsen rmlarsen commented Nov 22, 2021

Replace usage of deprecated Eigen class MappedSparseMatrix.

Description

Eigen::MappedSparseMatrix has been deprecated since Eigen 3.3 from 2016. Use the equivalent modern syntax Eigen::Map<Eigen::SparseMatrix<...>>.

The deprecated class was removed on the Eigen master branch in:

https://gitlab.com/libeigen/eigen/-/commit/7e586635ba85ba926ba3148f8d14beca3535f970

Suggested changelog entry:

Replace usage of deprecated ``Eigen::MappedSparseMatrix`` with ``Eigen::Map<Eigen::SparseMatrix<...>>`` for Eigen 3.3+.

Eigen::MappedSparseMatrix has been deprecated since Eigen 3.3 from 2016. Use the equivalent modern syntax Eigen::Map<Eigen::SparseMatrix<...>>.
@Skylion007
Copy link
Collaborator

Currently, the minimum supported Eigen version in PyBind11 is 3.2.7. Did this API exist in 3.2? Otherwise, we need to either raise the minimum Eigen version or add an ifdef backport.

@rmlarsen
Copy link
Contributor Author

rmlarsen commented Nov 22, 2021

@Skylion007 I don't think this existed in Eigen 3.2. What would you prefer, raising the minimum version or an ifdef? Eigen 3.2 is pretty ancient.

@rmlarsen
Copy link
Contributor Author

@Skylion007 what do you prefer?

@Skylion007
Copy link
Collaborator

If it's not too much trouble to enable a backport with a "using"/typedef, it would be nice to keep support for Eigen 3.2.

@rmlarsen
Copy link
Contributor Author

@Skylion007 PTAL

@Skylion007 Skylion007 requested review from rwgk and henryiii November 22, 2021 20:03
include/pybind11/eigen.h Outdated Show resolved Hide resolved
Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

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

Thanks all!
I'll go ahead merge this now.

@rmlarsen I'll import this into the Google env asap.

@rwgk rwgk merged commit 70a58c5 into pybind:master Nov 23, 2021
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Nov 23, 2021
@rmlarsen
Copy link
Contributor Author

@rwgk Thanks!

@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Dec 21, 2021
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.

4 participants