-
Notifications
You must be signed in to change notification settings - Fork 26
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
ClusterBasedNormalizer
code cleanup
#704
Conversation
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #704 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 17 17
Lines 1799 1800 +1
=========================================
+ Hits 1799 1800 +1
☔ View full report in Codecov by Sentry. |
Note: the lines |
data = np.stack([recovered_data, data[:, -1]], axis=1) # noqa: PD013 | ||
else: | ||
data = recovered_data | ||
recovered_data = np.stack([recovered_data, data[:, -1]], axis=1) # noqa: PD013 |
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.
Can we get rid of this noqa?
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.
Nope. It thinks that is pandas stack
and tells you to use melt
, which is a method numpy doesn't have.
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.
Is np.concatenate
the same ?
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.
stack
concatenates them along a different axis. It's also supposed to be faster. I think we could change it to np.concatenate([[array1], [array2]], axis=1)
, or something like that, but that just seemed more confusing than the noqa
.
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.
LGTM!
data = np.stack([recovered_data, data[:, -1]], axis=1) # noqa: PD013 | ||
else: | ||
data = recovered_data | ||
recovered_data = np.stack([recovered_data, data[:, -1]], axis=1) # noqa: PD013 |
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.
Is np.concatenate
the same ?
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.
Resolve #696.