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

Improve Windows compatibility. #82

Merged
merged 14 commits into from
Jul 16, 2019
Merged

Improve Windows compatibility. #82

merged 14 commits into from
Jul 16, 2019

Conversation

bdice
Copy link
Collaborator

@bdice bdice commented Jul 13, 2019

With help from @lmcinnes, I specialized the ripser class in C++ to fix build problems with Windows.

This PR uses MSVC instead of MINGW on Appveyor, enhancing compatibility with mainstream Windows. This should also make it possible to enable Windows builds via conda-forge.

@bdice bdice requested a review from sauln July 13, 2019 19:03
@bdice bdice self-assigned this Jul 13, 2019
@codecov
Copy link

codecov bot commented Jul 13, 2019

Codecov Report

Merging #82 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #82   +/-   ##
=======================================
  Coverage   92.41%   92.41%           
=======================================
  Files           3        3           
  Lines         145      145           
  Branches       24       24           
=======================================
  Hits          134      134           
  Misses          9        9           
  Partials        2        2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5cdc1e1...997672c. Read the comment docs.

@bdice bdice requested a review from lmcinnes July 13, 2019 20:36
@lmcinnes
Copy link

Looks good to me.

@ctralie
Copy link
Member

ctralie commented Jul 13, 2019

This is so great, thank you so much @bdice and @lmcinnes. If this works, it will save me tons of time re-implementing a numba version of ripser.

I am noticing this problem in the CI however:

c:\projects\ripser-py\ripser\ripser.cpp(885): error C2039: 'back_inserter': is not a member of 'std'

I'm surprised by this; why wouldn't this be imported in the std library? Is there a similar function that does the same thing?

@bdice
Copy link
Collaborator Author

bdice commented Jul 14, 2019

@ctralie Ah! I didn't see this on my local Windows build for some reason -- might be a difference in Visual Studio versions? As suggested in erengy/anitomy#2, I added #include <iterator>. I think this might fix it, we'll wait for CI to see.

@sauln sauln merged commit fbb5cdd into master Jul 16, 2019
@sauln sauln deleted the fix/windows-packing branch July 16, 2019 14:20
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