New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DM-7615: Support pytest #17
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that y[...] -> zz[...]
in tractor.py
, and flake8 is correct that setting the ellipse
variable in examples/shapeletBases.py
is unnecessary can be removed.
Other than that, all looks good, but I'd prefer to switch from putting the random seed in setUpClass
to setUp
if doing so doesn't lead to any immediate test failures (and if it does, that's a bigger problem).
tests/test_functorKeys.py
Outdated
|
||
class FunctorKeyTestCase(lsst.shapelet.tests.ShapeletTestCase): | ||
|
||
@classmethod | ||
def setUpClass(cls): | ||
np.random.seed(500) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this preserves the old behavior, it might be better to move this to setUp
instead so we get deterministic behavior for test methods individually. That's especially true if the order in which test methods are executed might not be preserved when we switch to pytest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I was wondering which way you wanted to go on that.
This adds pytest support and also renames the test files to allow test discovery.
I have made some flake8 fixes but there are a couple of issues that I'm not sure about:
For the example I'm not sure if the object is being retained for a reason. The
tractor.py
plotting code looks to be completely broken. I'm not sure whether it should be usingz[]
rather thany[]
.Looking at the history of the code and change c17fe67 from @TallJimbo, I think that the
y[]
should bez[]
.