-
Notifications
You must be signed in to change notification settings - Fork 127
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
General update to bring the main branch up to date #1471
Conversation
You should also update |
Done @coruscating |
The test needs to check for imports of any modules in the sklearn package, not just "import sklearn".
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.
Looks good. Some of the pylint "fixes" don't make the code better in my opinion but they are the easiest fixes (setting some variable to None
that can never be None
but will never not be set to the right value before it is used).
@@ -16,7 +16,7 @@ jobs: | |||
runs-on: ${{ matrix.os }} | |||
strategy: | |||
matrix: | |||
python-version: [3.8, "3.12"] | |||
python-version: [3.9, "3.12"] |
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 think this leaves an orphaned 3.8 job on the PR but we can ignore than when merging.
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.
which one? All the tests seem 3.9 or 3.12 (as far as I can tell)
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 branch protection rules were expecting a 3.8 test so it didn't show this PR as ready to merge. I've manually fixed them at 3.9 and 3.12.
@@ -121,6 +121,8 @@ def __init__( | |||
delay_ops = [delay.operation for delay in interleaved_element.get_instructions("delay")] | |||
if delay_ops: | |||
timing = BackendTiming(backend) | |||
else: | |||
timing = None |
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.
Yuck, this makes pylint happy but doesn't look any better to me (since the only use of timing
below fails if it is None
).
Summary
A few main changes