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

Fall back to CMake's Development package for Python in case of errors #2210

Merged
merged 7 commits into from
Nov 15, 2021
Merged

Fall back to CMake's Development package for Python in case of errors #2210

merged 7 commits into from
Nov 15, 2021

Conversation

Helveg
Copy link
Contributor

@Helveg Helveg commented Nov 12, 2021 •
edited

Fixes #2209 and any other environment in which Development.Module might not work, but Development would. It was a bit optimistic to hope that only the CMake version would ever be a problem, and this PR makes sure that whatever happens we revert to the default behavior.

@Helveg Helveg mentioned this pull request Nov 12, 2021
Closed
@jougs jougs added I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: High Should be handled next T: Enhancement New functionality, model or documentation labels Nov 15, 2021
@jougs jougs added this to In progress in PyNEST via automation Nov 15, 2021
@jougs jougs added this to the NEST 3.2 milestone Nov 15, 2021
@jougs jougs self-requested a review November 15, 2021 09:00
jougs
jougs approved these changes Nov 15, 2021
View reviewed changes
Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

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

This looks much better. Thanks for this change!

PyNEST automation moved this from In progress to Review Nov 15, 2021
@jougs jougs requested a review from ackurth November 15, 2021 09:03
@Helveg
Copy link
Contributor Author

Helveg commented Nov 15, 2021

Now that I look at it again, if everything goes well there won't be any output, should we add

else()
  find_package( Python 3.8 REQUIRED Interpreter Development.Module )
endif()

to get the output in the logs?

jougs
jougs reviewed Nov 15, 2021
View reviewed changes
cmake/ProcessOptions.cmake Outdated Show resolved Hide resolved
@Helveg @jougs
Added loud find_package if quiet one succeeds.
533b458 
Co-authored-by: Jochen Martin Eppler <jougs@gmx.net>
jougs
jougs reviewed Nov 15, 2021
View reviewed changes
cmake/ProcessOptions.cmake Outdated Show resolved Hide resolved
ackurth
ackurth approved these changes Nov 15, 2021
View reviewed changes
Copy link
Contributor

@ackurth ackurth left a comment

Choose a reason for hiding this comment

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

Great, works and is exactly the information I was lacking.

@jougs
Copy link
Contributor

jougs commented Nov 15, 2021

Thanks for the faster-than light reactions to my rolling reviews 😸

@jougs jougs changed the title Fall back to Development in case of any errors Fall back to Development Python module in case of any errors Nov 15, 2021
@jougs jougs merged commit 3ef911a into nest:master Nov 15, 2021
PyNEST automation moved this from Review to Done Nov 15, 2021
@Helveg Helveg deleted the fix/issue-2209 branch November 15, 2021 11:52
@jougs jougs mentioned this pull request Nov 22, 2021
Closed
@terhorstd terhorstd changed the title Fall back to Development Python module in case of any errors Fall back to Development Cmake module for Python detection Jan 21, 2022
@jougs jougs changed the title Fall back to Development Cmake module for Python detection Fall back to CMake's Development module for Python in case of errors Jan 21, 2022
@jougs jougs changed the title Fall back to CMake's Development module for Python in case of errors Fall back to CMake's Development package for Python in case of errors Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jougs jougs jougs approved these changes

@ackurth ackurth ackurth approved these changes

Assignees
No one assigned
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: High Should be handled next T: Enhancement New functionality, model or documentation
Projects
PyNEST
  
Done
Milestone
NEST 3.2
Development

Successfully merging this pull request may close these issues.

Problems with Development.Module change
3 participants
@Helveg @jougs @ackurth