Skip to content
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

Remove unit_params_range from generate.py #3121

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

JoeZiminski
Copy link
Collaborator

closes #3077 by removing the unused unit_params_range in generate.py. This previously allowed the user to set the parameters for templates injected into synthetic recordings. However I think it was superceeded by unit_params. @samuelgarcia you were correct on #3077 unit_params can take a tuple in which case it will use these as bounds to generate template parameters randomly. So unit_params performs the job of unit_params_range.

@alejoe91 alejoe91 added the generators Related to generator tools label Jul 2, 2024
@alejoe91 alejoe91 added this to the 0.101.0 milestone Jul 2, 2024
@zm711
Copy link
Collaborator

zm711 commented Jul 2, 2024

Any chance this needs a deprecation? Or is this new enough that it can just be deleted without annoying users?

@JoeZiminski
Copy link
Collaborator Author

Good question, my impression is it is quite new and it is not widely used. Also, it was not connected to anything so if was used it wasn't doing anything. As such, I think either remove or raise an error if trying to use saying it doesn't do anything, use unit_params instead, and remove version after next. Happy with both approaches 🤔

@samuelgarcia
Copy link
Member

Merci Joe
I am agree that we could remove this without warning it was somhow a hidden option that nobody used.

@alejoe91 alejoe91 merged commit 32a3bb4 into SpikeInterface:main Jul 3, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generators Related to generator tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose unit_params_range in test file generation
4 participants