-
Notifications
You must be signed in to change notification settings - Fork 62
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
Cleaning reconst scripts part2 #924
Cleaning reconst scripts part2 #924
Conversation
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.
Just a few minor comments! Good job.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #924 +/- ##
==========================================
+ Coverage 69.38% 69.46% +0.07%
==========================================
Files 389 389
Lines 20997 20989 -8
Branches 3231 3225 -6
==========================================
+ Hits 14568 14579 +11
+ Misses 5114 5102 -12
+ Partials 1315 1308 -7
|
fa83075
to
93ce647
Compare
Hello @EmmaRenauld, Thank you for updating ! There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2024-02-29 21:31:15 UTC |
Finally I added a bit more changes!
Overall, I thus have mainly small changes, and 4 new functions (created without changing any line of code!). I was not always sure where to put them though. |
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.
Some quick comments. LGTM
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.
Quick fix and then GTG
355cf70
to
c32176a
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.
Seems GTG !
Quick description
Part 2 in my meticulous verification of reconst scripts. Read the 9 next scripts in our excel list!
Almost all already correct! Changes a few small code style along the way. The only real changes:
- Removed the "useful for 2D data" in the help. We don't manage the 2D. We manage 3D data with one dimension set to 1, but we don't add the 3rd dimension ourselves.
- Changed the method it calls so that we don't send
args
. Will be easier to create a script.That's it!
...
Type of change
Check the relevant options.
Provide data, screenshots, command line to test (if relevant)
...
Checklist