aboutcode icon indicating copy to clipboard operation
aboutcode copied to clipboard

Replace and rewrite "assert" statements in non-test code

Open pombredanne opened this issue 11 months ago • 31 comments

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

pombredanne avatar Jan 12 '25 16:01 pombredanne

Heyyy, I'd like to work on this issue. Can you assign it to me?

TirthPanchal avatar Jan 12 '25 16:01 TirthPanchal

so, assert statements should be removed from all the repos of aboutcode-org., or only from aboutcode-org/scancode-toolkit ?

coderhuBypassion avatar Jan 13 '25 08:01 coderhuBypassion

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

Dhruv276R avatar Jan 14 '25 05:01 Dhruv276R

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.

Gurleen-kansray avatar Jan 15 '25 16:01 Gurleen-kansray

@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?

Dhruv276R avatar Jan 15 '25 16:01 Dhruv276R

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!

ONKARBH avatar Jan 17 '25 12:01 ONKARBH

@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

pombredanne avatar Jan 17 '25 14:01 pombredanne

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!

ONKARBH avatar Jan 17 '25 16:01 ONKARBH

@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 ?

pombredanne avatar Jan 17 '25 16:01 pombredanne

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!

ONKARBH avatar Jan 17 '25 16:01 ONKARBH

My bad: this is at https://github.com/aboutcode-org/aboutcode-toolkit/pull/574

pombredanne avatar Jan 17 '25 18:01 pombredanne

Is this issue still open?

Pratz2005 avatar Jan 26 '25 08:01 Pratz2005

Which directories are test directories? Is it anywhere mentioned? Asking to avoid them.

shafi456 avatar Jan 31 '25 07:01 shafi456

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: @.***>

Shivam11702 avatar Jan 31 '25 07:01 Shivam11702

@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

shafi456 avatar Jan 31 '25 09:01 shafi456

@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?

LSUDOKO avatar Feb 09 '25 19:02 LSUDOKO

is this issue still open ?? @pombredanne

shiv343 avatar Feb 11 '25 09:02 shiv343

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

akshatchauhan1 avatar Feb 15 '25 16:02 akshatchauhan1

@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

pombredanne avatar Feb 17 '25 13:02 pombredanne

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

akshatchauhan1 avatar Feb 19 '25 08:02 akshatchauhan1

@pombredanne Please assign this issue to me

Jaishree2310 avatar Feb 26 '25 15:02 Jaishree2310

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!

RasikaTelase avatar Apr 07 '25 19:04 RasikaTelase

Is this issue still open?

shubhika123 avatar Jul 23 '25 11:07 shubhika123

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 avatar Sep 12 '25 13:09 rajatsingh535

@rajatsingh535 which pull request?

pombredanne avatar Sep 23 '25 06:09 pombredanne

Hello @pombredanne, Here is the link to the pull request: https://github.com/aboutcode-org/container-inspector/pull/61

rajatsingh535 avatar Sep 23 '25 10:09 rajatsingh535

@pombredanne please assign me.

revaarathore11 avatar Oct 21 '25 11:10 revaarathore11

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

Bhoomi-bombom avatar Nov 02 '25 14:11 Bhoomi-bombom

hello @pombredanne i would like to work on this ??

santhosh-7777 avatar Dec 04 '25 08:12 santhosh-7777