Replace and rewrite "assert" statements in non-test code
See https://www.reddit.com/r/learnpython/comments/zd217y/is_using_assert_considered_good_practice/
assert statements should only be used for debugging purposes. They are removed when not running in debug mode (i.e. when invoking the Python command with the -O or -OO options).
Instead use a raise statement to throw a proper, informative runtime exception, if you need to. That would definitely be best practice when there'd otherwise be no sensible return value for the function.
We are using a few asserts in our main code. See https://github.com/search?q=org%3Aaboutcode-org+NOT+path%3Atest++"assert"++language%3APython&type=code
Heyyy, I'd like to work on this issue. Can you assign it to me?
so, assert statements should be removed from all the repos of aboutcode-org., or only from aboutcode-org/scancode-toolkit ?
Hey, I'd like to work on this issue. Can you assign this to me? And the assert statements are to be removed from all the repos or a specific one? @pombredanne
Hi @pombredanne , I have started working on this issue. I will replace the assert statements with raise statements in the main code (outside test files). I'll submit a PR soon. Let me know if there are specific repos other than scancode-toolkit where this change is needed.
@pombredanne I have made a PR on this https://github.com/aboutcode-org/aboutcode-toolkit/pull/574 Check and let me know for further enhacements. Do you need me to changes in other repos as well?
Hi @pombredanne, I would like to work on this issue to replace the assert statements with proper raise exceptions in the main code (outside test files). Could you please assign this issue to me? Let me know if any specific guidelines or repos need to be considered. Thank you!
@TirthPanchal re:
Heyyy, I'd like to work on this issue. Can you assign it to me?
we do not "assign" issue per se. You are welcome to work on this, but you want to sync up here with others to avoid double, duplicated work!
@Dhruv276R re:
Hey, I'd like to work on this issue. Can you assign this to me?
As explained above, we cannot "assign" work. You guys need to coordinate and collaborate to avoid stepping on each other.
And the assert statements are to be removed from all the repos or a specific one?
Rather replaced in all the repos, and only asserts that are not for tests, and only asserts that are for production code that could be ignored from a -O python run.
@Gurleen-kansray re:
Hi , I have started working on this issue. I will replace the assert statements with raise statements in the main code (outside test files). I'll submit a PR soon. Let me know if there are specific repos other than scancode-toolkit where this change is needed.
As said above, all repos, but not in test code: there we use "assert" and pytest does an awesome job assert rewriting job, so we surely do not want to change this.
@ONKARBH re:
Hi, I would like to work on this issue to replace the assert statements with proper raise exceptions in the main code (outside test files). Could you please assign this issue to me? Let me know if any specific guidelines or repos need to be considered. Thank you!
Same as above: all repos, not the tests, and you need to coordinate here with others
Hi everyone, I’d like to contribute to this issue by focusing on specific files/modules for replacing assert statements outside the test files. Please let me know how I can best coordinate with others to avoid overlap. Thank you!
@Dhruv276R re:
I have made a PR on this https://github.com/aboutcode-org/aboutcode-toolkit/pull/574 Check and let me know for further enhacements. Do you need me to changes in other repos as well?
This is awesome, but I cannot see the PR at https://github.com/aboutcode-org/aboutcode/pulls/574 ?
Hi @Dhruv276R, your work is great! However, the PR link seems incorrect. Can you confirm if the PR is in the aboutcode-toolkit repo and share the correct link? Thank you!
My bad: this is at https://github.com/aboutcode-org/aboutcode-toolkit/pull/574
Is this issue still open?
Which directories are test directories? Is it anywhere mentioned? Asking to avoid them.
they are mentioned like test_something
On Fri, 31 Jan 2025 at 12:45 PM, shafi456 @.***> wrote:
Which directories are test directories? Is it anywhere mentioned? Asking to avoid them.
— Reply to this email directly, view it on GitHub https://github.com/aboutcode-org/aboutcode/issues/175#issuecomment-2626462090, or unsubscribe https://github.com/notifications/unsubscribe-auth/BOUE56ECDJTQ6P5BMQWSTL32NMPJPAVCNFSM6AAAAABVBFNNJ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRWGQ3DEMBZGA . You are receiving this because you commented.Message ID: @.***>
@pombredanne Hey I fixed the issue for the extracode repo. Can you check if that's ok? https://github.com/aboutcode-org/extractcode/pull/71
@pombredanne Hey, I'd like to work on this issue. Can you assign this to me? And the assert statements are to be removed from all the repos or a specific one?
is this issue still open ?? @pombredanne
See https://www.reddit.com/r/learnpython/comments/zd217y/is_using_assert_considered_good_practice/
assert statements should only be used for debugging purposes. They are removed when not running in debug mode (i.e. when invoking the Python command with the -O or -OO options).
Instead use a raise statement to throw a proper, informative runtime exception, if you need to. That would definitely be best practice when there'd otherwise be no sensible return value for the function.
We are using a few asserts in our main code. See https://github.com/search?q=org%3Aaboutcode-org+NOT+path%3Atest++"assert"++language%3APython&type=code
I'd like to add my contribution to it
@shafi456
@pombredanne Hey I fixed the issue for the extracode repo. Can you check if that's ok? aboutcode-org/extractcode#71 This is a good start. See my comments there
For everyone, please read: https://aboutcode.readthedocs.io/en/latest/contributing/writing_good_commit_messages.html
https://github.com/aboutcode-org/aboutcode/compare/main...akshatchauhan1:aboutcode:akshatchauhan1-patch-1 you can check it out here whether it is fixed correctly or not @pombredanne
@pombredanne Please assign this issue to me
This PR refactors non-test code by replacing all usage of assert statements with proper error handling or conditional checks.
Purpose:
Improves code reliability and prevents unexpected termination in production. Scope:
No changes to test files. Affects only logic files using assert. Let me know if further changes are needed!
Is this issue still open?
Hello @pombredanne,
I have submitted this pull request to resolve issue #175. This is my first contribution to the project, and I'm very excited to be involved with AboutCode.
I believe I have correctly addressed the issue by replacing the assert statements with proper if...raise exceptions. I would be very grateful if you could please take a look at my pull request and let me know if it is correct. I am hopeful it can be merged so I can continue to contribute.
Thank you for your time!
@rajatsingh535 which pull request?
Hello @pombredanne, Here is the link to the pull request: https://github.com/aboutcode-org/container-inspector/pull/61
@pombredanne please assign me.
hello @pombredanne ,
I’ve submitted a pull request to resolve the issue by replacing the assert statements with proper if...raise exceptions. This is my first contribution to AboutCode, and I’m excited to get involved!
I’d really appreciate it if you could review the PR and let me know if everything looks good.
Thank you
hello @pombredanne i would like to work on this ??