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 print statements within functions #7337

Closed
CaedenPH opened this issue Oct 16, 2022 · 6 comments
Closed

Remove print statements within functions #7337

CaedenPH opened this issue Oct 16, 2022 · 6 comments
Labels
enhancement This PR modified some existing files

Comments

@CaedenPH
Copy link
Contributor

Feature description

Use flake8-print plugin to check for print statements and remove them if found within variable scopes

@CaedenPH CaedenPH added the enhancement This PR modified some existing files label Oct 16, 2022
@cclauss
Copy link
Member

cclauss commented Oct 16, 2022

I think this will be difficult to explain to new contributors.

@tianyizheng02
Copy link
Contributor

I've run flake8 with flake8-print and the print statements I've checked are all in if __name__ == "__main__" blocks. They're mainly just used for running functions on user inputs. I assume these print statements should stay?

@cclauss
Copy link
Member

cclauss commented Oct 18, 2022

Yes, they can remain as they are. CONTRIBUTING.md says that algorithmic functions should not input(), print(), plot, or read/write to files. However, there might be __main__, or main(), or other utility functions that do these things.

@dhruvmanila
Copy link
Member

Instead, I propose to make the following refactor:

For all the classes that provide some method to print out a representation of that object, use __str__ method to return the string representation instead. The caller then can do whatever he/she wants, print it, etc.

@SwaggyRajput
Copy link

Hello sir, can u assign me this issue. I would love to grab this opportunity.

@cclauss
Copy link
Member

cclauss commented Oct 22, 2022

Please stop waiting. We do not assign issues in this repo. Instead, we review pull requests so if you see something that is worth fixing then please create a pull request to fix it. #7499 (merged) seems to have solved most of these problems.

@TheAlgorithms TheAlgorithms deleted a comment from Anshika91 Oct 23, 2022
@cclauss cclauss closed this as completed Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This PR modified some existing files
Projects
None yet
Development

No branches or pull requests

5 participants