Tree tutorials translated into Python. #16620
Conversation
| sin, | ||
| cos, | ||
| sqrt, | ||
| ) |
There was a problem hiding this comment.
For consistency this parenthesis should be aligned under sqrt
There was a problem hiding this comment.
Thank you for this contribution!
I started adding some comments here and there (all about code style and commented-out-code basically) but then I realized there's just too many files and it'd get repetitive.
I'll summarize my comments in: I believe before adding these tutorials we should also clean them up a bit in the process: at the very least by removing all commented out code (or explaining why it's there) and using a consistent style to aid readability.
Also, I'm not sure it's a good idea to add all the tutorials in the same PR, and especially not in the same commit. Ideally each tutorial should have its own commit, or at least be grouped in smaller chunks.
I know it's quite a bit of work, but this is code that many users will use as an example to write their own programs, so it should be as clean as possible!
|
|
||
| # utils | ||
| def to_c( ls ): | ||
| return (c_double * len(ls) )( * ls ) |
There was a problem hiding this comment.
I would add a newline between the functions for readability. Also please use consistent spacing for the parentheses (prefer (ls) to ( ls ) and so on)
| # | ||
| # - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | ||
| # | ||
| print( "Geting and Compiling libEvent.so with ACLiC.") |
There was a problem hiding this comment.
Typo: "Geting".
Also see comment above for consistent parentheses spacing
| ], | ||
|
|
||
| check = True, | ||
|
|
There was a problem hiding this comment.
Remove empty newline here
| ROOTSYS = str( gROOT.GetDataDir() ) | ||
| # | ||
| for event_file in event_files: | ||
| #os.rename( event_file, ROOTSYS + "/include/" + event_file ) |
There was a problem hiding this comment.
Remove commented-out code
|
|
||
| # | ||
| # Moving Event* files to root/include directory. | ||
| # There was no other option, AddIncludePaths, AddDynamicPath weren't working well. |
There was a problem hiding this comment.
This comment doesn't look like it should belong to a tutorial. If we can't figure out why those alternative didn't work, we should either figure it out or not mention them
| #gSystem.CompileMacro( source_Event, "gO ") | ||
| gSystem.CompileMacro( source_Event, ) | ||
|
|
||
| # libEven.so |
There was a problem hiding this comment.
Remove all this commented-out code
| #gSystem.GetDynamicPath( ) | ||
|
|
||
|
|
||
| #import sys |
| gSystem.AddDynamicPath( "./Event" ) | ||
| # No need. It seems ACLiC already does that. | ||
| # | ||
| #ROOT.gSystem.Load("libEvent.so") # Inherited from .C Version. |
There was a problem hiding this comment.
If unneeded, please remove
| TH1F, | ||
| TGraph, | ||
| TLatex, | ||
| ) |
There was a problem hiding this comment.
Consistently align parentheses
|
|
||
| #utils | ||
| def to_c( ls ): | ||
| return (c_double * len(ls) )( * ls ) |
There was a problem hiding this comment.
See comment in the file above (spacing, newlines)
This Pull request:
Adds to the
root/tutorials/*.Cmacros their respective python scripts.Enjoy this "pyroothonic" syntax style.