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

Update Timing Parameters #799

Merged
merged 2 commits into from
Oct 11, 2018
Merged

Conversation

LukasNickel
Copy link
Member

  • use container for output
  • use geometry, peakpos and hillas_parameters as input
  • adapt test

@maxnoe
Copy link
Member

maxnoe commented Oct 10, 2018

We just rebased our stuff onto master, why was the merge needed?

@maxnoe
Copy link
Member

maxnoe commented Oct 10, 2018

@LukasNickel is a new master student in Dortmund, btw.

* use container for output
* use geometry, peakpos and hillas_parameters as input
* adapt tests
@codecov
Copy link

codecov bot commented Oct 10, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #799      +/-   ##
==========================================
+ Coverage   71.92%   71.94%   +0.01%     
==========================================
  Files         204      204              
  Lines       11039    11038       -1     
==========================================
+ Hits         7940     7941       +1     
+ Misses       3099     3097       -2
Impacted Files Coverage Δ
ctapipe/visualization/tests/test_mpl.py 100% <100%> (ø) ⬆️
ctapipe/image/tests/test_timing_parameters.py 100% <100%> (ø) ⬆️
ctapipe/io/containers.py 100% <100%> (ø) ⬆️
ctapipe/image/timing_parameters.py 100% <100%> (+6.06%) ⬆️

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 05ceb05...c3510f3. Read the comment docs.

@maxnoe
Copy link
Member

maxnoe commented Oct 10, 2018

I'm cannot explain why coverage of tools/camdemo.py went down. How is this possible?

@dneise
Copy link
Member

dneise commented Oct 11, 2018

I'm cannot explain why coverage of tools/camdemo.py went down. How is this possible?

Looked into this a little bit, with Max. The missed part is the body of the update() function, which is the func argument of FuncAnimation.

To us this smells a bit like a possible issue with the mpl backend ...

GUI testing is just hard (DNs personal opinion)

@maxnoe
Copy link
Member

maxnoe commented Oct 11, 2018

@LukasNickel can you fix the stuff that codacy complains about?

@LukasNickel
Copy link
Member Author

@LukasNickel can you fix the stuff that codacy complains about?

Going to do that asap.
Do we just not worry about the camdemo coverage? I can't see any changes in the file at all.

@maxnoe
Copy link
Member

maxnoe commented Oct 11, 2018

Do we just not worry about the camdemo coverage? I can't see any changes in the file at all.

Yes, not related to this PR.

* removed unused imports
* fixed whitespace issues
@kosack
Copy link
Contributor

kosack commented Oct 11, 2018

Do we just not worry about the camdemo coverage? I can't see any changes in the file at all.

that seems to be a false positive, so just ignore it. Not sure why it came up in this PR.

@kosack kosack merged commit 268c390 into cta-observatory:master Oct 11, 2018
@LukasNickel LukasNickel deleted the update_timing branch October 11, 2018 10:17
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.

4 participants