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

Add new DecFileParser class, with test suite #21

Merged
merged 13 commits into from
Apr 5, 2019

Conversation

eduardo-rodrigues
Copy link
Member

With this new DecFileParser class I was able to trivially parse and check the master LHCb DECAY.DEC file:

from decaylanguage.dec.dec import *
import time

start = time.time()
p = DecFileParser('decaylanguage/data/DECAY_LHCB.DEC')
p.parse()
end = time.time()
print('Parsing time = %s seconds.' % (end-start))

The massive file is nicely parsed in under 2 seconds :-).

And, BTW, I found a little bug in the file - duplication of decay definition for the Sigma(1775)0 particle! LHCb simulation group about to be informed ...

@eduardo-rodrigues
Copy link
Member Author

I will be adding a bit more doc, in particular. But the code is otherwise up for discussion, @henryiii.

@eduardo-rodrigues eduardo-rodrigues changed the title Add new DecFileNotParsed class, with test suite Add new DecFileParser class, with test suite Apr 2, 2019
@eduardo-rodrigues eduardo-rodrigues marked this pull request as ready for review April 4, 2019 10:48
@eduardo-rodrigues
Copy link
Member Author

@henryiii, this is now ready for review, though I will still add some more documentation, and am also thinking about an extra method to display a la dot-file a decay chain, for example.

@coveralls
Copy link

coveralls commented Apr 4, 2019

Coverage Status

Coverage remained the same at 0.0% when pulling abb8e20 on eduardo-decfileparser-intro into 6369662 on master.

@henryiii
Copy link
Member

henryiii commented Apr 5, 2019

Can you rebase to see if this starts passing? Looks good otherwise. I might recommend a __slots__ just for clarity.

@henryiii henryiii force-pushed the eduardo-decfileparser-intro branch from 1a1804e to 062837c Compare April 5, 2019 09:02
@henryiii henryiii force-pushed the eduardo-decfileparser-intro branch 3 times, most recently from 1458457 to acc8f74 Compare April 5, 2019 10:24
@henryiii henryiii force-pushed the eduardo-decfileparser-intro branch from acc8f74 to 8447fc6 Compare April 5, 2019 10:32
Copy link
Member

@henryiii henryiii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More docs and things would be good, though I'm okay to go ahead and merge, then add to it afterwards in a new PR.

@eduardo-rodrigues
Copy link
Member Author

Thanks a lot for the look and the improvements/fixes.

I've just rebased.

@eduardo-rodrigues
Copy link
Member Author

Forgot to say: I do want to add more docs, and tests, though I put a significant amount of it already. I will merge, then, as this PR is rather large and contains some nice additions.

Thanks.

@eduardo-rodrigues eduardo-rodrigues merged commit 3f6151c into master Apr 5, 2019
@eduardo-rodrigues eduardo-rodrigues deleted the eduardo-decfileparser-intro branch April 19, 2019 12:45
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.

None yet

3 participants