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

Remove outdated tools #1827

Merged
merged 5 commits into from
Jan 31, 2022
Merged

Remove outdated tools #1827

merged 5 commits into from
Jan 31, 2022

Conversation

maxnoe
Copy link
Member

@maxnoe maxnoe commented Jan 25, 2022

Remove the four tools discussed for removal here #1506

@codecov
Copy link

codecov bot commented Jan 25, 2022

Codecov Report

Merging #1827 (f203b00) into master (6bd131a) will increase coverage by 0.42%.
The diff coverage is n/a.

❗ Current head f203b00 differs from pull request most recent head fee7ba2. Consider uploading reports for the commit fee7ba2 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1827      +/-   ##
==========================================
+ Coverage   91.68%   92.10%   +0.42%     
==========================================
  Files         190      186       -4     
  Lines       15004    14902     -102     
==========================================
- Hits        13756    13726      -30     
+ Misses       1248     1176      -72     
Impacted Files Coverage Δ
ctapipe/reco/hillas_reconstructor.py 98.47% <ø> (ø)
ctapipe/reco/impact.py 44.98% <ø> (+0.19%) ⬆️
ctapipe/tools/tests/test_tools.py 100.00% <ø> (ø)
ctapipe/visualization/mpl_camera.py 91.21% <0.00%> (-0.24%) ⬇️
ctapipe/image/toymodel.py 100.00% <0.00%> (ø)
ctapipe/coordinates/__init__.py 100.00% <0.00%> (ø)
ctapipe/image/tests/test_toy.py 100.00% <0.00%> (ø)
ctapipe/core/tests/test_traits.py 100.00% <0.00%> (ø)
ctapipe/image/tests/test_hillas.py 100.00% <0.00%> (ø)
ctapipe/io/tests/test_toysource.py 100.00% <0.00%> (ø)
... and 60 more

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 6bd131a...fee7ba2. 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.

while I still use some of these for debugging and outreach, I think it's ok to drop them since now with the DL1/DL2 data model + ctapipe.io.TableLoader + Jupyter notebooks, it's easy to replicate all of this functionality, and often simpler.

@kosack
Copy link
Contributor

kosack commented Jan 25, 2022

Though I still like camdemo, as it's fun and a nice simple example of writing a Tool and using the InstrumentDescription model!

@maxnoe
Copy link
Member Author

maxnoe commented Jan 27, 2022

Though I still like camdemo, as it's fun and a nice simple example of writing a Tool and using the InstrumentDescription model!

Then should I move it to examples? Or the Tool documentation?

I am against having something that's just supposed to be a documentation example installed in PATH along with the "real" tools.

@kosack kosack merged commit d1ff3d5 into master Jan 31, 2022
@maxnoe maxnoe deleted the remove_tools branch January 31, 2022 17:46
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.

3 participants