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

Add unit to h_max in HillasReconstructor #797

Merged
merged 2 commits into from Oct 10, 2018

Conversation

Projects
None yet
3 participants
@jjlk
Contributor

jjlk commented Oct 10, 2018

Hi @mackaiver , @kosack ,
This short PR add a unit (meter) to the height of the shower mawimum computed by estimate_h_max (for consistency with other methods).

Don't you want also to rename the python file: HillasReconstructor.py to hillas_reconstructor.py for consistency with the other files?

@kosack

kosack approved these changes Oct 10, 2018

@mackaiver

This comment has been minimized.

Member

mackaiver commented Oct 10, 2018

Don't you want also to rename the python file: HillasReconstructor.py to hillas_reconstructor.py for consistency with the other files?

Yup. Renaming is a good idea. 👍

@codecov

This comment has been minimized.

codecov bot commented Oct 10, 2018

Codecov Report

Merging #797 into master will decrease coverage by 0.16%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #797      +/-   ##
==========================================
- Coverage   71.92%   71.76%   -0.17%     
==========================================
  Files         204      204              
  Lines       11039    11039              
==========================================
- Hits         7940     7922      -18     
- Misses       3099     3117      +18
Impacted Files Coverage Δ
ctapipe/reco/HillasReconstructor.py 97% <100%> (ø) ⬆️
ctapipe/reco/tests/test_HillasReconstructor.py 94.59% <100%> (ø) ⬆️
ctapipe/tools/camdemo.py 58.97% <0%> (-23.08%) ⬇️
ctapipe/utils/unstructured_interpolator.py 64.7% <0%> (ø) ⬆️
ctapipe/flow/sequential/producer_sequential.py 16.21% <0%> (ø) ⬆️

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 3152788...eb1c084. Read the comment docs.

@kosack kosack merged commit 05ceb05 into cta-observatory:master Oct 10, 2018

3 checks passed

codecov/patch 100% of diff hit (target 71.92%)
Details
codecov/project Absolute coverage decreased by -0.16% but relative coverage increased by +28.07% compared to 3152788
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

watsonjj added a commit to watsonjj/ctapipe that referenced this pull request Nov 9, 2018

Merge branch 'master' into bokeh_plotter
* master: (60 commits)
  Add test that shows slicing breaks cam geom and fix it (cta-observatory#782)
  fix ctapipe build failure (cta-observatory#811)
  fix package name for yaml (should be pyyaml) (cta-observatory#810)
  Implement number of islands (cta-observatory#801)
  fixed ranges of cam-display so they correspond to fixed toymodel sims (cta-observatory#808)
  Fix unknown section example warning (cta-observatory#800)
  Fix timing parameters for case when there are negative values in image (cta-observatory#804)
  Update Timing Parameters (cta-observatory#799)
  speed up unit tests that use test_event fixture (cta-observatory#798)
  Add unit to h_max in HillasReconstructor (cta-observatory#797)
  Codacy code style improvements (cta-observatory#796)
  Minor changes: mostly deprecationwarning fixes (cta-observatory#787)
  Array plotting (cta-observatory#784)
  added a config file for github change-drafter plugin (cta-observatory#795)
  Simple HESS adaptations (cta-observatory#794)
  add test for sliced geometries for hillas calculation (cta-observatory#781)
  Impact intersection (cta-observatory#778)
  updated main documentation page (cta-observatory#792)
  Implement concentration image features (cta-observatory#791)
  Fix bad builds by changing channel name (missing pyqt package) (cta-observatory#793)
  ...

# Conflicts:
#	ctapipe/calib/camera/dl1.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment