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.

Sign up for GitHub

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

Jump to bottom

[POSSIBLY REDUNDANT] Cxx17 compatibility fixes for the Rosetta fork of RDKit. #3

Open
wants to merge 2 commits into
base: Rosetta
Choose a base branch
from
Open

[POSSIBLY REDUNDANT] Cxx17 compatibility fixes for the Rosetta fork of RDKit. #3

wants to merge 2 commits into from

Conversation

vmullig
Copy link
Member

@vmullig vmullig commented May 24, 2024

The Rosetta fork of RDKit currently fails to compile on Mac/clang with extras=cxx17 (or extras=masala, which turns on Cxx17 compilation). This PR removes some instances of the deprecated std::unary_function and std::binary_function, switching them to std::function. Notes on this:

  1. This may not be the best way to make the switch.
  2. This may have already been fixed in a better way in the main RDKit repo, in which case we should just close this PR and update to their version.

@roccomoretti

@vmullig
Fixes to rdkit for cxx17 compatibility -- replace std::binary_functio…
e37a157 
…n and std::unary_function with std::function.
@vmullig vmullig added the bug Something isn't working label May 24, 2024
@vmullig vmullig requested a review from roccomoretti May 24, 2024 17:47
@vmullig vmullig self-assigned this May 24, 2024
@vmullig vmullig changed the base branch from master to Rosetta_2021_03_05 May 24, 2024 17:48
@vmullig vmullig changed the base branch from Rosetta_2021_03_05 to Rosetta May 24, 2024 17:49
@vmullig
Adding comments clearly marking stuff that was modified in the Rosett…
4de2485 
…a fork of the RDKit project to allow Cxx17 compatibility while maintaining Cxx11 compatibility. These things could result in merge conflicts in the future, when the primary RDKit repo is merged into this one, and those conflicts should be resolved in favour of the primary fork.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roccomoretti roccomoretti Awaiting requested review from roccomoretti

Assignees

@vmullig vmullig

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@vmullig