-
Notifications
You must be signed in to change notification settings - Fork 6
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 to new commondata names - validphys #2074
Conversation
What's missing here? (I think the errors are just the fact that the plots have changed as well, right?) |
Yes, but so far I fixed far from all mentions of the old names so there is still a lot to do I'm afraid |
19f0629
to
95c6be3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peterkrack for some of tests to pass, you need to rebase this on top of master. While for the figures (in test_plots
), you need to re-generate them.
I tried to replace ATLASTTBARTOT with ATLAS_TTBAR_7TEV_TOT_X-SEC because I did not find any corresponding dataset among the new_commondata set names. test_plots.py fails with the new dataset. I am getting NotImplementedError: The process type INC for ATLAS_TTBAR_7TEV_TOT_X-SEC is not implemented. If I remove the ATLAS dataset from test_plots.py the test passes, at least when running it on stoomboot. |
This one can be solved by simply rebasing, and indeed it is now.
It shouldn't be a problem which datasets you use to check the plotting. I you use |
e01aa94
to
2caba59
Compare
I've rebased on top of master and updated the regression tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested the tests but haven't gone over every change of name in detail, so I'm just trusting they are sensible.
Just the one comment about the author in the tests (I've also opened the following issue #2130 related to that)
@@ -1,7 +1,7 @@ | |||
meta: | |||
title: PDF flavours plot example | |||
author: Rosalyn Pearson | |||
keywords: [example] | |||
author: Rosalyn L. Pearson |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would honestly change the authors in the example from any to Lazy Person
or The Creator
or some other funny thing like that. Mainly so that if people uploads a report without changing these fields we know that we don't know who uploaded it instead of thinking it was Rosalyn :__
87a6f29
to
0917135
Compare
After dealing with the dataset names in n3fit in #2031, this PR continues to deal with the vp side.
closes #1969