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

Add test that shows slicing breaks cam geom and fix it #782

Merged
merged 5 commits into from
Oct 26, 2018

Conversation

maxnoe
Copy link
Member

@maxnoe maxnoe commented Sep 10, 2018

No description provided.

ctapipe/instrument/tests/test_camera.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 22, 2018

Codecov Report

Merging #782 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #782      +/-   ##
==========================================
+ Coverage   71.92%   71.93%   +0.01%     
==========================================
  Files         204      204              
  Lines       11056    11061       +5     
==========================================
+ Hits         7952     7957       +5     
  Misses       3104     3104
Impacted Files Coverage Δ
ctapipe/instrument/tests/test_camera.py 100% <100%> (ø) ⬆️
ctapipe/instrument/camera.py 97% <100%> (ø) ⬆️

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 02fbcdb...ac8b926. Read the comment docs.

@maxnoe
Copy link
Member Author

maxnoe commented Oct 10, 2018

merge? We just went into this again.

@@ -6,6 +6,8 @@
_get_min_pixel_seperation,
)
import pytest
from ctapipe.io.eventsourcefactory import EventSourceFactory
from ctapipe.utils import get_dataset_path
Copy link
Contributor

Choose a reason for hiding this comment

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

is get_dataset_path or EventSourceFactory used anywhere in this test? Otherwise remove it

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@maxnoe
Copy link
Member Author

maxnoe commented Oct 11, 2018

done

@maxnoe maxnoe mentioned this pull request Oct 17, 2018
@maxnoe
Copy link
Member Author

maxnoe commented Oct 17, 2018

@kosack See #802, would be nice if this gets merged

@kosack kosack merged commit 56411ae into cta-observatory:master Oct 26, 2018
@maxnoe maxnoe deleted the fix_cam_rotation branch October 26, 2018 11:39
watsonjj added a commit to watsonjj/ctapipe that referenced this pull request Nov 9, 2018
* 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants