Skip to content

TST, MAINT: Replace most setup with setup_method (also teardown) #22487

Merged
charris merged 5 commits intonumpy:mainfrom
seberg:setup-fixup
Oct 27, 2022
Merged

TST, MAINT: Replace most setup with setup_method (also teardown) #22487
charris merged 5 commits intonumpy:mainfrom
seberg:setup-fixup

Conversation

@seberg
Copy link
Copy Markdown
Member

@seberg seberg commented Oct 27, 2022

In some cases, the replacement is clearly not what is intended,
in those (where setup was called explicitly), I mostly renamed
setup to _setup.
The test_ccompile_opt is a bit confusing, so left it right now
(this will probably fail)


Also changed the docs a little bit. (Draft, just because I expect one failure still.)

Closes gh-22486

Not actually sure that setup_module() is what was wanted here, but
it works?
Mention a bit more about actual pytest fixtures.
In some cases, the replacement is clearly not what is intended,
in those (where setup was called explicitly), I mostly renamed
`setup` to `_setup`.
The `test_ccompile_opt` is a bit confusing, so left it right now
(this will probably fail)
len(m1) - reduce(lambda x, y:x + y, m1))
assert_(eq(xm, xf))
assert_(eq(filled(xm, 1.e20), xf))
assert_(eq(x, xm))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old code called the setup manually between loop iterations, so just refactored this to a fixture.

self._setup()

def setup(self):
def _setup(self):
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the init calls it explicitly, this seemed right. There is only one test, so it doesn't matter anyway, but I suspect the intention isn't to rerun it on each test method.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And hopefully we can remove distutils in a few years :)


def teardown(self):
def teardown_method(self):
np.seterr(**self.olderr)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Admittedly, these might be nicer as @np.errstate(invalid="ignore") decorators.

@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Oct 27, 2022
@seberg seberg marked this pull request as ready for review October 27, 2022 16:03
@seberg
Copy link
Copy Markdown
Member Author

seberg commented Oct 27, 2022

Seems to be working, so considering it done. I did not review the replacements very carefully, so we could do that. But I think they should be safe and seem mostly right (even if often there are probably better ways to do things.)

@charris
Copy link
Copy Markdown
Member

charris commented Oct 27, 2022

The Cygwin failure looks unrelated, I suspect the chocolatey v1.2.0 release on Oct 19. @DWesl Thoughts?

@DWesl
Copy link
Copy Markdown
Contributor

DWesl commented Oct 27, 2022

That seems to have flipped between 21 hours ago and seven hours ago; I'm not sure how much delay there is in propagating chocolatey updates to mirrors. I could try switching to cygwin/cygwin-install-action@v2 rather than egor-tensin/setup-cygwin@v3, which should at least produce a different error.

@charris
Copy link
Copy Markdown
Member

charris commented Oct 27, 2022

@DWesl the log explicitly mentions chocolatey v1.2.0, but it is still a guess on my part that is the cause. But it looks suspicious.

@charris
Copy link
Copy Markdown
Member

charris commented Oct 27, 2022

LGTM modulo a couple of easy lint fixes.

@DWesl
Copy link
Copy Markdown
Contributor

DWesl commented Oct 27, 2022

And twenty-one hours ago, the CI job was using 1.1.0, so that looks like the problem.

#22488 changes from using chocolatey to setup-x86_64.exe

Not new things, but in touched lines...
@charris charris merged commit 7596d43 into numpy:main Oct 27, 2022
@charris
Copy link
Copy Markdown
Member

charris commented Oct 27, 2022

Thanks Sebastian.

@charris charris added 03 - Maintenance and removed 09 - Backport-Candidate PRs tagged should be backported labels Oct 28, 2022
@charris charris changed the title TST,MAINT: Replace most setup with setup_method (also teardown) TST, MAINT: Replace most setup with setup_method (also teardown) Oct 28, 2022
@seberg seberg deleted the setup-fixup branch October 28, 2022 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TST,MAINT: pytest 8 phases out some nose "plugin" backwards compatibility

3 participants