-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Close ZipFile in compression test #22679
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
Close ZipFile in compression test #22679
Conversation
Hello @TomAugspurger! Thanks for submitting the PR.
|
@Zac-HD any thoughts on this hypothesis failures?
We've set that value in Line 13 in c040353
|
Ah, not quite - you need to assign to hypothesis.settings.register_profile(
"ci",
suppress_health_check=(hypothesis.HealthCheck.too_slow,)
)
hypothesis.settings.load_profile("ci") might be less error-prone, and easier to add to in future! |
Codecov Report
@@ Coverage Diff @@
## master #22679 +/- ##
=======================================
Coverage 92.17% 92.17%
=======================================
Files 169 169
Lines 50715 50715
=======================================
Hits 46747 46747
Misses 3968 3968
Continue to review full report at Codecov.
|
@@ -105,6 +105,7 @@ def get_exceldf(self, basename, ext, *args, **kwds): | |||
class ReadingTestsBase(SharedItems): | |||
# This is based on ExcelWriterBase | |||
|
|||
@td.skip_if_no('xlrd', '1.0.1') # GH-22682 |
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.
Could we alternately just bump the minimum version of xlrd? 1.0.0 is over 2 years old at this point. Only 1.1.0 has been released since then but even that is over a year old now
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.
Thought about it, though this seemed easier for now. I don't have a strong preference.
@TomAugspurger - FYI HypothesisWorks/hypothesis#1572 will ensure that settings issue can't come back. |
Merging this in an hour if there are no objections, as this is causing issues on master. For now I'm ok with just skipping those tests @WillAyd, but I'm not at all against bumping our excel-related minimum versions and doing a bit of a cleanup (in a separate PR 😄). |
* Updates HypothesisCheck setting * Skips tests for old xlrd
* Updates HypothesisCheck setting * Skips tests for old xlrd
Maybe fixes #22675
Maybe fixed #22682