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

Change default durations argument from list in generate.py #2342

Open
JoeZiminski opened this issue Dec 15, 2023 · 2 comments
Open

Change default durations argument from list in generate.py #2342

JoeZiminski opened this issue Dec 15, 2023 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@JoeZiminski
Copy link
Collaborator

JoeZiminski commented Dec 15, 2023

Just leaving here so I don't forget, happy to make a PR on this. Some of the default arguments for functions in generate.py are mutable, which is not reccomended due to pythons crazy behaviour. These could be changed to None and the default value set in the body of the function.

@JoeZiminski JoeZiminski changed the title Change default argument durations argument from list in generate_recordings.py Change default argument durations argument from list in generate.py Dec 15, 2023
@JoeZiminski JoeZiminski changed the title Change default argument durations argument from list in generate.py Change default durations argument from list in generate.py Dec 15, 2023
@h-mayorquin
Copy link
Collaborator

h-mayorquin commented Dec 18, 2023

Yeah, I noticed that too recently. This is a common mistake that people make in Python. Altought we don't really mutate those lists is would be a good practice to change it. A good heuristic to remember this is that default values are values (not expressions).

I made one for the other instances of this pattern in #2345.

@h-mayorquin
Copy link
Collaborator

This discussion came up again:

#2777

@alejoe91 really wanted to move forward with this, @samuelgarcia supported the idea of using tuples or copying the list at the beginning of the function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants