HEXRD / hexrd

A cross-platform, open-source library for the analysis of X-ray diffraction data.
Other
54 stars 26 forks source link

Remove unnecessary pass statements #670

Closed kevindlewis23 closed 5 days ago

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 30.11%. Comparing base (b32cf68) to head (d1d9057).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #670 +/- ## ========================================== - Coverage 30.16% 30.11% -0.06% ========================================== Files 139 139 Lines 22551 22437 -114 ========================================== - Hits 6803 6757 -46 + Misses 15748 15680 -68 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kevindlewis23 commented 1 week ago

Please be careful when removing pass statements, as they're sometimes used in important ways to indicate that a function or class is not implemented. Without a function definition, Python will throw errors.

Passes are not required for abstract methods, as long as there's a docstring. I already tested this and no errors are thrown. From looking online, this is seems to be the preferred way to do it. https://stackoverflow.com/questions/45826692/body-of-abstract-method-in-python-3-5

ZackAttack614 commented 1 week ago

While this is true, relying on Python's evaluation of the docstring doesn't provide the user with much feedback if they're using a method that hasn't yet been overridden. Best practice is to raise a NotImplementedError in methods rather than do nothing, so that if a user calls a method that does not have a real definition, the system doesn't break in a way that is hard to understand.

kevindlewis23 commented 1 week ago

While this is true, relying on Python's evaluation of the docstring doesn't provide the user with much feedback if they're using a method that hasn't yet been overridden. Best practice is to raise a NotImplementedError in methods rather than do nothing, so that if a user calls a method that does not have a real definition, the system doesn't break in a way that is hard to understand.

Okay, I'll raise a NotImplementedError then