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

read dl2 and sim info into pyirf internal format #521

Merged
merged 9 commits into from
Sep 28, 2020

Conversation

vuillaut
Copy link
Member

@vuillaut vuillaut commented Sep 28, 2020

I will add a unit test

@vuillaut vuillaut marked this pull request as ready for review September 28, 2020 12:57
@codecov
Copy link

codecov bot commented Sep 28, 2020

Codecov Report

Merging #521 into master will increase coverage by 0.17%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #521      +/-   ##
==========================================
+ Coverage   41.48%   41.65%   +0.17%     
==========================================
  Files          77       77              
  Lines        6501     6520      +19     
==========================================
+ Hits         2697     2716      +19     
  Misses       3804     3804              
Impacted Files Coverage Δ
lstchain/io/io.py 80.71% <100.00%> (+0.71%) ⬆️
lstchain/scripts/tests/test_lstchain_scripts.py 98.75% <100.00%> (+0.10%) ⬆️

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 43951ff...470e2f0. Read the comment docs.

@vuillaut vuillaut merged commit faa75c7 into cta-observatory:master Sep 28, 2020
@vuillaut vuillaut deleted the io_pyirf branch September 28, 2020 15:14
@maxnoe
Copy link
Member

maxnoe commented Sep 28, 2020

Maybe working on a pyirf branch until everything is ready might be a good idea instead of merging pieces into the master?

@rlopezcoto
Copy link
Contributor

I agree with @maxnoe, specially now that we are finalizing the tests on the data before releasing v0.6.1, it would be nice not to include in the master any non-necessary changes not to break any compatibilities and have reproducible results

@moralejo
Copy link
Collaborator

For me tests are not passing, it complains about missing pyirf.

@chaimain
Copy link
Contributor

Yes, the setup file also has to be updated, and as @maxnoe suggested, we will work on a separate branch to work on our scripts, until both lstchain and pyirf are ready for the DL3 scripts.

@moralejo
Copy link
Collaborator

Ok, please revert these changes. Amazingly I cannot even run the R0 to DL1 step - though it should not at all be related to IRFs, I guess

@moralejo
Copy link
Collaborator

moralejo commented Sep 28, 2020

Actually it is a bit urgent, would be nice if sb can fix this today

@rlopezcoto
Copy link
Contributor

@moralejo I guess that you need to update the environment with:
conda env update -n lst-dev -f environment.yml

does that work?

@moralejo
Copy link
Collaborator

@moralejo I guess that you need to update the environment with:
conda env update -n lst-dev -f environment.yml

does that work?

Yes, thanks! I thought pip install would do it...

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.

6 participants