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

Change misleading script names, solves #329 #334

Merged
merged 6 commits into from
Apr 8, 2020

Conversation

rlopezcoto
Copy link
Contributor

Following #329, I changed:

  • dl0_to_dl1.py => r0_to_dl1.py
  • lstchain_mc_dl1_to_dl2.py => lstchain_dl1_to_dl2.py
  • lstchain_data_muon_analysis_dl1.py => lstchain_muon_analysis.py

and the scripts/tests/notebooks where they were called

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

@codecov
Copy link

codecov bot commented Apr 8, 2020

Codecov Report

Merging #334 into master will increase coverage by 0.70%.
The diff coverage is 53.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #334      +/-   ##
==========================================
+ Coverage   42.88%   43.59%   +0.70%     
==========================================
  Files          70       69       -1     
  Lines        4694     4618      -76     
==========================================
  Hits         2013     2013              
+ Misses       2681     2605      -76     
Impacted Files Coverage Δ
lstchain/reco/r0_to_dl1.py 69.60% <ø> (ø)
lstchain/scripts/lstchain_data_r0_to_dl1.py 0.00% <0.00%> (ø)
lstchain/scripts/lstchain_dl1_muon_analysis.py 0.00% <ø> (ø)
lstchain/scripts/lstchain_dl1_to_dl2.py 0.00% <ø> (ø)
lstchain/scripts/lstchain_mc_r0_to_dl1.py 0.00% <0.00%> (ø)
lstchain/scripts/lstchain_mc_r0_to_dl2.py 0.00% <0.00%> (ø)
lstchain/io/tests/test_io.py 100.00% <100.00%> (ø)
lstchain/mc/tests/test_senstivity.py 93.61% <100.00%> (ø)
lstchain/reco/__init__.py 100.00% <100.00%> (ø)
lstchain/scripts/tests/test_lstchain_scripts.py 100.00% <100.00%> (ø)
... and 3 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 7422c50...c882e5c. Read the comment docs.

maxnoe
maxnoe previously approved these changes Apr 8, 2020
morcuended
morcuended previously approved these changes Apr 8, 2020
@maxnoe
Copy link
Member

maxnoe commented Apr 8, 2020

Maybe keep the dl1 in the muon script? Just for clarity? lstchain_dl1_muon_analysis

@rlopezcoto rlopezcoto dismissed stale reviews from morcuended and maxnoe via c882e5c April 8, 2020 08:57
@maxnoe maxnoe self-requested a review April 8, 2020 08:58
@maxnoe maxnoe merged commit 98e270b into cta-observatory:master Apr 8, 2020
@rlopezcoto rlopezcoto deleted the solve_misleading_names branch April 9, 2020 10:58
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