Skip to content

[uss_qualifier] Don't try to cleanup None area in down_uss#1423

Closed
the-glu wants to merge 1 commit intointeruss:mainfrom
Orbitalize:fix_1422
Closed

[uss_qualifier] Don't try to cleanup None area in down_uss#1423
the-glu wants to merge 1 commit intointeruss:mainfrom
Orbitalize:fix_1422

Conversation

@the-glu
Copy link
Copy Markdown
Contributor

@the-glu the-glu commented Apr 17, 2026

#1405 removed by mistake the None check that seems to appear in some cases. This fixes #1422

@BenjaminPelletier
Copy link
Copy Markdown
Member

It seems like the root issue here should have been caught by basedpyright. The reason it didn't is that the argument of interest didn't have any type annotations, so I would expect that to be a core fix for this problem.

By putting the conditional inside the method, we are effectively making a method that may do nothing when called, and that seems surprising to me. In one of the uses:

        self.begin_test_step("Clear operational intents created by virtual USS")
        self._clear_op_intents(estimated_max_extents)
        self.end_test_step()

...it wouldn't make sense to perform that test step at all if _clear_op_intents was going to be a no-op -- beginning and ending a test step while knowing nothing would happen in it is confusing. So, I don't think we want to no-op the method when the argument is None, I think we should ensure the argument is not None before calling the method. I have #1425 as an alternate approach.

@the-glu the-glu closed this Apr 17, 2026
@the-glu
Copy link
Copy Markdown
Contributor Author

the-glu commented Apr 17, 2026

It seems like the root issue here should have been caught by basedpyright. The reason it didn't is that the argument of interest didn't have any type annotations, so I would expect that to be a core fix for this problem.

By putting the conditional inside the method, we are effectively making a method that may do nothing when called, and that seems surprising to me.

Yes I do agree, #1425 is better.. I would expect basedpyright to do some type inference, at least "something that is not None", but it seems that not the case ;/

@BenjaminPelletier
Copy link
Copy Markdown
Member

I think basedpyright continues to demonstrate its value as it would have caught this one if we had been more diligent about type annotations. It would be cool if it did type inferences, but it's still very useful as-is in my opinion

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AttributeError: 'NoneType' object has no attribute 'to_f3548v21' in DownUSS.cleanup()

2 participants