hnn-core
hnn-core copied to clipboard
show users how to delete drives
Fairly self-explanatory: it's currently not shown in our documentation how users can delete drives.
I thought we were trying to push users toward creating a whole new Network
instance?
you can just do del net.external_drives[drive_name]
. That's easier, no?
Sure, but the connectivity needs to be cleared as well. See here for an example. I know we discussed on numerous occasions the possibility of packaging all this in a simple net.remove_drive()
function, however, it's my recollection that we opted for the solution that promoted a more ligthweight API: just recreate your network.
I think it's worth giving users a simple function if they can't do it non-trivially in a few lines of code.
See: #550
Hello @jasmainak @rythorpe, I am Rajat Partani. I wanted to apply for GSOC 2023 with HNN and am searching for a good starting point. If this issue is not resolved yet, can I work on it? I am fairly new to open source, I will be really thankful for your guidance.
go for it.
Hi @jasmainak, I had a few queries regarding what to do
- There is a clear_drives function in network.py. Is documentation required for this. If yes can you please help me with the exact location where documentation needs to be updated
- In #550 clear drivers is modified to delete only selected drives but was never merged. Is some work required there
- In gui there is a button for deleting a drive. Is documentation required for that
I think #550 needs to be finished. It's basically a small function to remove the connectivity and the drive itself. But tests are remaining. The test should check that the drive is indeed removed. Look at the other tests and the contributing guide to see how we do tests. Can you start a new PR from the branch in #550 ?
Okay will work on the test section. If many rebase errors come up with #550 can I replicate the changes in the latest branch of master, write the tests and make a single PR?
If you're comfortable with cherry picking, I'd recommend branching from master and then cherry picking the single commit in #550. Then you won't have to worry about rebasing if you run into conflicts in the test files.
+1 for using chery-pick !
Sure I'll go with cherry pick
Hey @jasmainak @rythorpe , I did cherry picking and was able to get the commit. Only some typo errors came up in the function.
- The function deletes all the external drives when no arguments are passed.
- In test_network.py after clearing drives #550 asserts length of connectivity as 0 instead of 50. In the old code before that commit, assertion was made to 50 which seems correct.
- Does this mean an external drive has multiple connectivities?
- Also there is only a single test where all the drives are deleted. Should I add more tests for deleting a custom number of external drives and then checking the length of connectivity.
- Here is a image where I have printed all values before and after running the function
Hey @jasmainak @rythorpe , I added some code to test custom delete drives
In network.py
In test_network.py
The driver names were correctly passed but self.connectivity shows a weird behavior. This time it became 0 after deleting all drives. Here are the print results
Please check this out
Does this mean an external drive has multiple connectivities?
You might want to read this: https://jonescompneurolab.github.io/hnn-under_the_hood/04_evoked-rhythmic-inputs/04_evoked-rhythmic-inputs
Should I add more tests for deleting a custom number of external drives and then checking the length of connectivity.
sure why not. But don't hard code the number of connections ... they should be computed somehow and intuitive to someone reading the code.
This time it became 0 after deleting all drives
Did you look what net.connectivity
contains?
Also, please don't share screenshots of code. You can share code in markdown with proper formatting.
I will look into net.connectivity. In future will share code using proper practices. Also is one custom drive deletion test sufficient or should I add more? Currently I deleted 2 drives in the first go and remaining all in the second go
the tests should check all the possible scenarios and edge cases ... it doesn't matter how many you add