-
Notifications
You must be signed in to change notification settings - Fork 28
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
Remove unused configuration settings #1346
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1346 +/- ##
==========================================
- Coverage 89.04% 89.03% -0.01%
==========================================
Files 76 76
Lines 10243 10243
==========================================
- Hits 9121 9120 -1
- Misses 1122 1123 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
tox.ini
Outdated
ignore:Passing None into shape arguments.*:DeprecationWarning:h5py | ||
ignore:the imp module is deprecated:DeprecationWarning | ||
ignore:`Unit` has been deprecated:DeprecationWarning:humanize | ||
ignore:The distutils package is deprecated:DeprecationWarning:joblib |
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.
are you sure we have minimal versions provided so that we no longer can run into those warningns?
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.
The warnings don't occur in testing anymore (as the tests pass), and this configuration only affects testing.
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.
Correct, but testing might be done where we do not have most recent versions of those dependencies (conda, or some linux distribution) thus causing fails.
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.
Most of our runtime dependencies don't have minimum versions specified at the moment, so theoretically testing elsewhere could use arbitrarily old packages with all sorts of warnings. Exactly what situation are you trying to avoid?
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.
pretty much those I described -- testing package build ATM on conda, but later could be on debian/ubuntu systems. If we don't have minimal versions specified, we have no guarantees that we would not hit those warnings which are to be ignored. So why not to keep them around for 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.
I've removed the commit that deleted the "ignores" and created #1360.
faf3fd1
to
3b5edb7
Compare
type checking was fixed elsewhere |
🚀 PR was released in |
No description provided.