-
Notifications
You must be signed in to change notification settings - Fork 2
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
Clean files in local_modules #131
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.
Hi Camilo! I've a couple of things I'd reintroduce (units on physical values) but otherwise this looks good!
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.
One final set of changes – otherwise looks good!
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.
Awesome! 🚀
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 meant to send this yesterday. Consider seeing if there's anything relevant left!
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.
Looks good to me!
all good here @cpaniaguam ? |
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 am approving but added a bunch of extra suggestions.
a = "%2.1e" % low | ||
b = "%2.1e" % high |
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.
Old style formatting.
a = "%2.1e" % low | |
b = "%2.1e" % high | |
a = f"{low:.1e}" | |
b = f"{high:.1e}" |
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.
file re-formatted
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.
This change didn't get applied?
|
||
return a[0:3] +'-'+ b +' Hz' | ||
return a[0:3] + "-" + b + " Hz" |
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.
return a[0:3] + "-" + b + " Hz" | |
return f"{a[0:3]}-{b} Hz" |
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.
file re-formatted
Co-authored-by: Carlos Paniagua <[email protected]>
Co-authored-by: Carlos Paniagua <[email protected]>
Co-authored-by: Carlos Paniagua <[email protected]>
…esat2-tracks into clean-files-modules
…esat2waves into clean-files-modules
@cpaniaguam I finished your last review. Check it again and let me know if I there anything I missed |
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 couple more easy changes.
Co-authored-by: Carlos Paniagua <[email protected]>
Co-authored-by: Carlos Paniagua <[email protected]>
…esat2waves into clean-files-modules
Co-authored-by: Carlos Paniagua <[email protected]>
@cpaniaguam Done! Check again. |
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.
Looks good!
I am not really sure why most of the conversations disappeared. They didn't appear in context so I had to have two windows opened, one at the conversations page and another with the file being reviewed. Also, in the future please consider using the add suggestion to batch
feature to co-author commits.
I did use the |
@hollandjg everything good on your side? |
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.
Looks good to me! 🚀
local_modules
folder