-
Notifications
You must be signed in to change notification settings - Fork 33
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
Fixed Notebooks DeadTime, Modeling,Window Functions #79
Fixed Notebooks DeadTime, Modeling,Window Functions #79
Conversation
-Fixed syntax errors/ value errors. -ps.freq and ps.power had different lengths, so truncated one array to match the length.
satshed the notebooks (as suggested ) and edited the specific cells that were causing issues
@matteobachetti, |
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.
Thanks @AnonymousCodes911!
A couple of points that need to be addressed:
- the window_functions notebook still has a lot of plot changes without significant code change, it should be cleaned up
- The modeling notebook should not need to filter a part of the array. If this is needed, it is a bug. I would avoid making this kind of change, which hides a possible bug and is not easily understandable from users reading the docs. Could you please eliminate this from the PR and open an Issue instead?
- Also, please do not explain your changes with comments in the notebooks, these messages should appear in the commit messages instead!
-removed obsolete seaborn dependencies - reverted changes that might be a (?bug)
@matteobachetti |
@AnonymousCodes911 Almost there! So, I currently see five files changed, and the Window one still has a lot of output (and you reverted the correct change 😅). Also, there still are comments that should not be there. Please check the current version of the "Files changed" tab (press "Refresh" if needed) to see all the changes you are proposing. |
e985812
to
f9c5ac5
Compare
This reverts commit 12abbc4.
…911/notebooks into new-NoteBookTests
@AnonymousCodes911 you have to make the diff with respect to our main, not your local copy. I suggest to clone the repo again in a separate directory, copy the window notebook, and make the small modification you are proposing in that one. I will squash and merge to eliminate all the intermediate steps. Otherwise, it might be quicker and cleaner to:
Please note that in the cross correlation notebook you fused two code lines: |
I created another PR😅, hopefully now the things will be in line, and it ll be much cleaner. |
-Fixed syntax errors/ value errors.
-ps.freq and ps.power had different lengths, so truncated one array to match the length.