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

Fix CALIB_SCALE key in analysis.yaml #153

Merged
merged 1 commit into from
Jul 20, 2021
Merged

Conversation

HealthyPear
Copy link
Member

Fix a typo in the example config file of the analysis which would disable CALIB_SCALE if not fixed by the user

@HealthyPear HealthyPear added wrong behaviour The code works but produces clearly wrong results fix A PR that fixes a bug or a wrong behaviour. labels Jul 15, 2021
@HealthyPear HealthyPear added this to the v0.5.0 milestone Jul 15, 2021
@codecov
Copy link

codecov bot commented Jul 15, 2021

Codecov Report

Merging #153 (4f73af1) into master (bc24635) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #153   +/-   ##
=======================================
  Coverage   66.93%   66.93%           
=======================================
  Files          23       23           
  Lines        2238     2238           
=======================================
  Hits         1498     1498           
  Misses        740      740           

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 bc24635...4f73af1. Read the comment docs.

Copy link
Contributor

@kosack kosack left a comment

Choose a reason for hiding this comment

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

this reminds me that if we use yaml config files, we should use a schema to validate them. But if all this will use ctapipe's config system in the future, there is no problem (it already does validation)

@HealthyPear
Copy link
Member Author

this reminds me that if we use yaml config files, we should use a schema to validate them. But if all this will use ctapipe's config system in the future, there is no problem (it already does validation)

sure, no need to make it fancier here (aside from these bugs which are anyway rare now...)

We just need to make sure that all the small important options that I add in protopipe from the CTAMARS comparison will be available from ctapipe-process

@HealthyPear HealthyPear merged commit 845189c into master Jul 20, 2021
@HealthyPear HealthyPear deleted the update_analysis_config branch July 20, 2021 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A PR that fixes a bug or a wrong behaviour. wrong behaviour The code works but produces clearly wrong results
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants