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

Observable template #2623

Merged
merged 2 commits into from
Mar 21, 2019
Merged

Observable template #2623

merged 2 commits into from
Mar 21, 2019

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Mar 21, 2019

sorry

patches #2621

@jngrad jngrad requested a review from fweik March 21, 2019 13:04
@fweik
Copy link
Contributor

fweik commented Mar 21, 2019

Why did the old code build?

@jngrad
Copy link
Member Author

jngrad commented Mar 21, 2019

Not sure, there isn't any T class/struct anywhere; you can even add using C = T; or using D = UWX; in /src/script_interface/observables/CylindricalProfileObservable.hpp and it will still compile just file. But it won't work in /src/core/. Maybe the -Wall flag in CMake is not inherited by the target that compiles the Python bindings?

@codecov
Copy link

codecov bot commented Mar 21, 2019

Codecov Report

Merging #2623 into python will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           python   #2623   +/-   ##
======================================
  Coverage      78%     78%           
======================================
  Files         495     495           
  Lines       27366   27366           
======================================
  Hits        21519   21519           
  Misses       5847    5847

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 4ad7bc4...6dc346e. Read the comment docs.

@jngrad
Copy link
Member Author

jngrad commented Mar 21, 2019

File /src/script_interface/observables/CylindricalProfileObservable.hpp needs to be removed. It is not included anywhere, and the classes declared in it are actually already declared in /src/script_interface/observables/initialize.cpp and properly unit tested.

@fweik fweik merged commit 337cddf into espressomd:python Mar 21, 2019
@jngrad jngrad deleted the CoreObs-typo branch January 18, 2022 12:11
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.

2 participants