-
Notifications
You must be signed in to change notification settings - Fork 52
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
[WIP] Make all plots with time end at stop #752
Conversation
Hey, I have now taken on this pull request. I'm thinking about the options for the deprecation cycle of defining tmin and tmax for plot_dipole et al.
I'm happy to make all of these clear in the tutorials when I'm done! |
I think we should not change behavior while deprecating ... tmin and tmax should be set to None by default. If it is not None, then raise deprecation warning telling the user that they should set directly with plt.xlim() and plt.ylim() to avoid this warning if it is None, then use the times attribute: extracellular.times, cell_response.times, dipole.times etc. do the Thanks for taking over the PR @katduecker ! |
@katduecker I am able to reproduce locally. Are you sure you have the editable import hnn_core
hnn_core.__version__
hnn_core.__path__ it should point to your github folder not to the anaconda folder. |
Did you look at the contributing guide ? You need to run the command: python setup.py build_mod You don't need two local installations :) Just switch branches on git ... once you have a developer install, you can switch between different versions simply by switching branches. In case you want to maintain a copy of hnn-core installation for reproducibility reasons, I would recommend creating a separate conda environment for your project and installing hnn-core there. |
Co-authored-by: Ryan Thorpe <[email protected]>
Co-authored-by: Ryan Thorpe <[email protected]>
1a672d4
to
fdc6335
Compare
@katduecker I think one reason the tests are failing is because the GUI code depends on some of these plotting functions: Try running |
@@ -106,25 +105,20 @@ def simulate_dipole(net, tstop, dt=0.025, n_trials=None, record_vsec=False, | |||
return dpls | |||
|
|||
|
|||
def _read_dipole_txt(fname, extension='.txt'): | |||
def _read_dipole_txt(fname): |
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.
@katduecker here is the problem! Since the time you had copied dipole.py
to your desktop the master branch has changed. Therefore these modifications are undoing the latest commits to the master branch.
I would reccomend copying this function exactly as it's written on the current master branch and replacing it in your code. That way the only changes that show up in the diff are related to the tstop
PR.
If done correctly you will no longer see _read_dipole_txt()
show up in the files changed.
@@ -179,6 +173,10 @@ def read_dipole(fname): | |||
The instance of Dipole class | |||
""" | |||
|
|||
# For supporting tests in test_gui.py | |||
if isinstance(fname, StringIO): | |||
return _read_dipole_txt(fname) |
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.
you will want to copy the read_dipole()
function from the master branch and replace it in your code as well
@katduecker I think I figured out the issue. If you're not planning on working on this over the weekend then we can sort things our in person on Monday. The cleanest solution would be to If you don't want to mess with the |
@ntolley thanks so much, that makes sense! I'll get everything ready for Monday up to the point where I'd commit the changes so that we could talk this over before I do to avoid further problems? |
Due to issues when resetting to the commits from this PR, I have opened a new PR #769 |
Continues the PR #683
Closes #544