BUG: Fix Series constructor when copy=True #15128
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jreback
reviewed
Jan 13, 2017
| y = pd.Series(x, copy=True, dtype=float) | ||
|
|
||
| # copy=True maintains original data in Series | ||
| self.assertTrue(x.equals(y)) |
There was a problem hiding this comment.
use tm.assert_series_equal to compare the series.
jreback
reviewed
Jan 13, 2017
| def test_constructor_copy(self): | ||
| # GH15125 | ||
| # test dtype parameter has no side effects on copy=True | ||
| x = Series(np.array([1.])) |
There was a problem hiding this comment.
do this in a loop, testing the for both of these as input to the Series.
[1.], np.array([1.])
jreback
reviewed
Jan 13, 2017
| @@ -362,3 +362,4 @@ Bug Fixes | |||
|
|
|||
|
|
|||
| - Bug in ``pd.read_csv()`` for the C engine where ``usecols`` were being indexed incorrectly with ``parse_dates`` (:issue:`14792`) | |||
| - Bug in ``Series`` constructor when both ``copy=True`` and ``dtype`` argument is provided (:issue:`15125`) | |||
|
minor comments. ping when updated / green. |
rouzazari
force-pushed
the
series_dtype_param_no_side_effects_when_copy_true
branch
from
221af87 to
f7af1c2
Compare
jreback
reviewed
Jan 14, 2017
|
|
||
| # changes to origin of copy does not affect the copy | ||
| x[0] = 2. | ||
| with self.assertRaises(AssertionError): |
There was a problem hiding this comment.
here you would want to
self.assertFalse(x.equals(y))
using assert_series_equal to test non-equality in anti-pattern.
jreback
reviewed
Jan 14, 2017
| x[0] = 2. | ||
| with self.assertRaises(AssertionError): | ||
| tm.assert_series_equal(x, y) | ||
| self.assertEqual(y[0], 1.) |
There was a problem hiding this comment.
add a self.assertEqual(x[0], 2.)
Updates Series constructor to include copy argument when dtype argument is also provided. Adds tests for copy parameter. xref pandas-dev#15125
rouzazari
force-pushed
the
series_dtype_param_no_side_effects_when_copy_true
branch
from
f7af1c2 to
7b0c959
Compare
Current coverage is 85.53% (diff: 100%)@@ master #15128 diff @@
==========================================
Files 145 145
Lines 51288 51288
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 43872 43871 -1
- Misses 7416 7417 +1
Partials 0 0
|
|
Thanks for pointing out the anti-pattern, @jreback. All green now. Let me know if you see any other changes. |
|
thanks! |
AnkurDedania
pushed a commit
to AnkurDedania/pandas
that referenced
this pull request
Mar 21, 2017
Updates Series constructor to include copy argument when dtype argument is also provided. closes pandas-dev#15125 Author: Rouz Azari <[email protected]> Closes pandas-dev#15128 from rouzazari/series_dtype_param_no_side_effects_when_copy_true and squashes the following commits: 7b0c959 [Rouz Azari] BUG: Fix Series constructor when copy=True
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates Series constructor to include copy argument when dtype argument
is also provided. Adds tests for copy parameter.
xref #15125
git diff upstream/master | flake8 --diff