The Wayback Machine - https://web.archive.org/web/20211029142606/https://github.com/TheAlgorithms/Python/pull/4699
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

Approve functions used as default arguments #4699

Merged

Conversation

2 participants
@cclauss
Copy link
Member

@cclauss cclauss commented Aug 30, 2021

Describe your change:

Use linter directive # noqa: B0008 for these functions because they set a seed value and BaseBackend is reused across all calls and LinearCongruentialGenerator.__init__() is only called once for each instance.

flake8-bugbear B008 Do not perform function calls in argument defaults. The call is performed only once at function definition time. All calls to your function will reuse the result of that definition-time function call. If this is intended, assign the function call to a module-level variable and use that variable as a default value.

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.
Copy link
Member

@dhruvmanila dhruvmanila left a comment

If this is intended, assign the function call to a module-level variable and use that variable as a default value.

Why don't we do what the linter is suggesting us to? I hate adding ignore directives throughout the code. If not, then we should atleast explain in a comment as to why that directive was added.

The default value for **seed** is the result of a function call which is not normally recommended and causes flake8-bugbear to raise a B008 error. However, in this case, it is accptable because `LinearCongruentialGenerator.__init__()` will only be called once per instance and it ensures that each instance will generate a unique sequence of numbers.
The default value for **backend** is the result of a function call which is not normally recommended and causes flake8-bugbear to raise a B008 error. However, in this case, it is accptable because `Aer.get_backend()` is called when the function is definition and that same backend is then reused for function calls.
Copy link
Member

@dhruvmanila dhruvmanila left a comment

I think the explanation should be a comment instead of adding in a docstring as it is intended only for developers instead of end-users, otherwise LGTM.

@cclauss
Copy link
Member Author

@cclauss cclauss commented Aug 31, 2021

Comment above function definition or below? If below, before docstring or after?

@dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Aug 31, 2021

Above the directive which means above the function definition.

@cclauss cclauss merged commit ef98271 into TheAlgorithms:master Aug 31, 2021
2 checks passed
@cclauss cclauss deleted the function-calls-as-default-arguments branch Aug 31, 2021
shermanhui added a commit to shermanhui/Python that referenced this issue Oct 22, 2021
* Approve functions used as default argumenets

* The default value for **seed** is the result of a function call

The default value for **seed** is the result of a function call which is not normally recommended and causes flake8-bugbear to raise a B008 error. However, in this case, it is accptable because `LinearCongruentialGenerator.__init__()` will only be called once per instance and it ensures that each instance will generate a unique sequence of numbers.

* The default value for **backend** is the result of a function call

The default value for **backend** is the result of a function call which is not normally recommended and causes flake8-bugbear to raise a B008 error. However, in this case, it is accptable because `Aer.get_backend()` is called when the function is definition and that same backend is then reused for function calls.

* Update linear_congruential_generator.py

* Update ripple_adder_classic.py

* Update ripple_adder_classic.py

* Update ripple_adder_classic.py

* Update ripple_adder_classic.py

* Update ripple_adder_classic.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment