DM-36472: Fix a bug in _validateGalsimInterpolant #17
Conversation
arunkannawadi
force-pushed
the
tickets/DM-36472
branch
2 times, most recently
from
297f783
to
b9dd883
Compare
arunkannawadi
force-pushed
the
tickets/DM-36472
branch
from
b9dd883
to
b12e2d9
Compare
|
Problems with GHA I guess. Linting check is not getting triggered. |
arunkannawadi
force-pushed
the
tickets/DM-36472
branch
from
b12e2d9
to
000ad7e
Compare
| @@ -33,8 +33,8 @@ | |||
|
|
|||
|
|
|||
| class PiffPsfDeterminerConfig(BasePsfDeterminerTask.ConfigClass): | |||
| def _validateGalsimInterpolant(name: str) -> bool: # noqa: N805 | |||
| """A helper function to validate the GalSim interpolant at config time. | |||
| def validateGalsimInterpolant(name: str) -> bool: # noqa: N805 | |||
There was a problem hiding this comment.
I still think this should be a private method (leading underscore) even though it is used in the tests.
Also, I don't know how this works as a static method without the @staticmethod decorator... or wait, it's because it's a local method, I guess, that just happens to work because it's at the class scope.
Recommendation: remove this from the class level, the check function should be outside the class (at line 33), and have a leading underscore. The noqa can then be removed and the scope will make more sense.
There was a problem hiding this comment.
Thanks Eli. Done. My rationale for making it a public method was so that the user could check for themselves whether a string is valid or not, without having to start a processing job. But one could also argue that that is conveyed in the docstring accurately.
| class PiffPsfDeterminerConfig(BasePsfDeterminerTask.ConfigClass): | ||
| def _validateGalsimInterpolant(name: str) -> bool: # noqa: N805 | ||
| """A helper function to validate the GalSim interpolant at config time. | ||
| def validateGalsimInterpolant(name: str) -> bool: |
There was a problem hiding this comment.
This should also be a private method with a leading underscore. Thanks!
arunkannawadi
force-pushed
the
tickets/DM-36472
branch
from
2dad655
to
245e7d9
Compare
arunkannawadi
force-pushed
the
tickets/DM-36472
branch
from
245e7d9
to
c9f52b7
Compare
arunkannawadi
changed the title
No description provided.