DEPR: remove previously-deprecated get_value, set_value
State: closed

Deprecated in 0.21.0

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Deprecate
description: Functionality to remove in pandas

Comments:
Note I have nothing against this PR itself, but I don't think we should be removing things after the RC.
that makes sense, will move note to the 0.26.0 file
(or, since there is no 0.26.0 file yet, will wait until there is)
> Any thought to migrating it to test_interals then? I can see the value of separating these out from other tests at the very least

Looks like these two methods are used in tests scattered across a handful of files.  Getting a better organization for indexing tests is worthwhile, but is a pretty big task
created a 0.26.0 file and moved the deprecation-removal there
Are we just pretending like we're doing 0.26.0 for now, and moving to 1.0
if we decide we're ready? I think that makes the most sense.

On Fri, Jul 19, 2019 at 10:02 AM jbrockmendel <notifications@github.com>
wrote:

> created a 0.26.0 file and moved the deprecation-removal there
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/27377?email_source=notifications&email_token=AAKAOIVJJRPM3O7EOVVZAYTQAHJPHA5CNFSM4ICW3Z3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2L4OFQ#issuecomment-513263382>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAKAOIUQBC2VDK7XDRTZLTLQAHJPHANCNFSM4ICW3Z3A>
> .
>

> Are we just pretending like we're doing 0.26.0 for now, and moving to 1.0 if we decide we're ready? I think that makes the most sense.

That's the impression I got from [dont remember, but I'm 80% sure it wasn't a dream]
Just added the milestone - can change name as required
> I see I think only [1 usage]

I count 15 non-test usages
> > I see I think only [1 usage]
> 
> I count 15 non-test usages

show me the list
Searching 1589 files for "\._get_value\(|\._set_value\(" (regex, case sensitive)

pandas/core/frame.py:
 2823          col = self.columns.get_loc(col)
 2824          index = self.index.get_loc(index)
 2825:         return self._get_value(index, col, takeable=True)
 2826  
 2827      def _set_value(self, index, col, value, takeable: bool = False):
 ....
 2845              if takeable is True:
 2846                  series = self._iget_item_cache(col)
 2847:                 return series._set_value(index, value, takeable=True)
 2848  
 2849              series = self._get_item_cache(col)
 ....
 3786              result = np.empty(n, dtype="O")
 3787              for i, (r, c) in enumerate(zip(row_labels, col_labels)):
 3788:                 result[i] = self._get_value(r, c)
 3789  
 3790          if is_object_dtype(result):

pandas/core/indexing.py:
  123              key = tuple(com.apply_if_callable(x, self.obj) for x in key)
  124              try:
  125:                 values = self.obj._get_value(*key)
  126              except (KeyError, TypeError, InvalidIndexError):
  127                  # TypeError occurs here if the key has non-hashable entries,
  ...
 1735          # a fast-path to scalar access
 1736          # if not, raise
 1737:         values = self.obj._get_value(*key)
 1738          return values
 1739  
 ....
 2035          # a fast-path to scalar access
 2036          # if not, raise
 2037:         values = self.obj._get_value(*key, takeable=True)
 2038          return values
 2039  
 ....
 2173  
 2174          key = self._convert_key(key)
 2175:         return self.obj._get_value(*key, takeable=self._takeable)
 2176  
 2177      def __setitem__(self, key, value):
 ....
 2188          key = list(self._convert_key(key, is_setter=True))
 2189          key.append(value)
 2190:         self.obj._set_value(*key, takeable=self._takeable)
 2191  
 2192  

pandas/core/sparse/frame.py:
  473              stacklevel=2,
  474          )
  475:         return self._get_value(index, col, takeable=takeable)
  476  
  477      def _get_value(self, index, col, takeable=False):
  ...
  481              series = self._get_item_cache(col)
  482  
  483:         return series._get_value(index, takeable=takeable)
  484  
  485      _get_value.__doc__ = get_value.__doc__
  ...
  517              stacklevel=2,
  518          )
  519:         return self._set_value(index, col, value, takeable=takeable)
  520  
  521      def _set_value(self, index, col, value, takeable=False):
  522:         dense = self.to_dense()._set_value(index, col, value, takeable=takeable)
  523          return dense.to_sparse(
  524              kind=self._default_kind, fill_value=self._default_fill_value

pandas/core/sparse/series.py:
  350  
  351      def _set_with_engine(self, key, value):
  352:         return self._set_value(key, value)
  353  
  354      def abs(self):
  ...
  412          )
  413  
  414:         return self._get_value(label, takeable=takeable)
  415  
  416      def _get_value(self, label, takeable=False):
  ...
  454              stacklevel=2,
  455          )
  456:         return self._set_value(label, value, takeable=takeable)
  457  
  458      def _set_value(self, label, value, takeable=False):
  ...
  461          # if the label doesn't exist, we will create a new object here
  462          # and possibly change the index
  463:         new_values = values._set_value(label, value, takeable=takeable)
  464          if new_values is not None:
  465              values = new_values

pandas/tests/frame/test_api.py:
   68          for idx in float_frame.index:
   69              for col in float_frame.columns:
   70:                 result = float_frame._get_value(idx, col)
   71                  expected = float_frame[col][idx]
   72                  tm.assert_almost_equal(result, expected)

pandas/tests/frame/test_constructors.py:
  330          frame_none = DataFrame(dict(a=None), index=[0])
  331          frame_none_list = DataFrame(dict(a=[None]), index=[0])
  332:         assert frame_none._get_value(0, "a") is None
  333:         assert frame_none_list._get_value(0, "a") is None
  334          tm.assert_frame_equal(frame_none, frame_none_list)
  335  
  ...
  701          for col in df.columns:
  702              for row in df.index:
  703:                 data.setdefault(col, {})[row] = df._get_value(row, col)
  704  
  705          result = DataFrame(data, columns=rng)
  ...
  709          for col in df.columns:
  710              for row in df.index:
  711:                 data.setdefault(row, {})[col] = df._get_value(row, col)
  712  
  713          result = DataFrame(data, index=rng).T

pandas/tests/frame/test_indexing.py:
 1850          for idx in float_frame.index:
 1851              for col in float_frame.columns:
 1852:                 result = float_frame._get_value(idx, col)
 1853                  expected = float_frame[col][idx]
 1854                  assert result == expected
 ....
 1905          for idx in float_frame.index:
 1906              for col in float_frame.columns:
 1907:                 float_frame._set_value(idx, col, 1)
 1908                  assert float_frame[col][idx] == 1
 1909  
 1910      def test_set_value_resize(self, float_frame):
 1911  
 1912:         res = float_frame._set_value("foobar", "B", 0)
 1913          assert res is float_frame
 1914          assert res.index[-1] == "foobar"
 1915:         assert res._get_value("foobar", "B") == 0
 1916  
 1917          float_frame.loc["foobar", "qux"] = 0
 1918:         assert float_frame._get_value("foobar", "qux") == 0
 1919  
 1920          res = float_frame.copy()
 1921:         res3 = res._set_value("foobar", "baz", "sam")
 1922          assert res3["baz"].dtype == np.object_
 1923  
 1924          res = float_frame.copy()
 1925:         res3 = res._set_value("foobar", "baz", True)
 1926          assert res3["baz"].dtype == np.object_
 1927  
 1928          res = float_frame.copy()
 1929:         res3 = res._set_value("foobar", "baz", 5)
 1930          assert is_float_dtype(res3["baz"])
 1931          assert isna(res3["baz"].drop(["foobar"])).all()
 1932          msg = "could not convert string to float: 'sam'"
 1933          with pytest.raises(ValueError, match=msg):
 1934:             res3._set_value("foobar", "baz", "sam")
 1935  
 1936      def test_set_value_with_index_dtype_change(self):
 ....
 1940          # so column is not created
 1941          df = df_orig.copy()
 1942:         df._set_value("C", 2, 1.0)
 1943          assert list(df.index) == list(df_orig.index) + ["C"]
 1944          # assert list(df.columns) == list(df_orig.columns) + [2]
 ....
 1951          # create both new
 1952          df = df_orig.copy()
 1953:         df._set_value("C", "D", 1.0)
 1954          assert list(df.index) == list(df_orig.index) + ["C"]
 1955          assert list(df.columns) == list(df_orig.columns) + ["D"]
 ....
 1965          df = DataFrame(index=index, columns=range(4))
 1966          with pytest.raises(KeyError, match=r"^0$"):
 1967:             df._get_value(0, 1)
 1968  
 1969      def test_single_element_ix_dont_upcast(self, float_frame):

pandas/tests/series/indexing/test_datetime.py:
  111      index = DatetimeIndex(dates)
  112  
  113:     s = Series()._set_value(dates[0], 1.0)
  114:     s2 = s._set_value(dates[1], np.nan)
  115  
  116      expected = Series([1.0, np.nan], index=index)

pandas/tests/series/indexing/test_indexing.py:
  373  def test_set_value(test_data):
  374      idx = test_data.ts.index[10]
  375:     res = test_data.ts._set_value(idx, 0)
  376      assert res is test_data.ts
  377      assert test_data.ts[idx] == 0
  ...
  379      # equiv
  380      s = test_data.series.copy()
  381:     res = s._set_value("foobar", 0)
  382      assert res is s
  383      assert res.index[-1] == "foobar"

37 matches across 9 files

@jbrockmendel 

ok I guess this is not trivial to remove. However, this does need rationalization (e.g. the name is not inline with *any* other indexers, even internally that we have). So this should be addressed at some point; these routines likely are not great at handling for example non-basic dtypes as these *never* got love and have been externally replaced by iat/at.

So if we don't have an issue can you create one.thanks.
@jbrockmendel if you can't tell, I really really dislike doing these private version of public methods just so we can deprecate them, rather than actually fix things. This is just a propagation of technical debt and *really* hurting the project (just a bit of venting, you are certainly addressing lots of this prior techinical debt).
ping on green.
Definitely agreed on getting rid of the private versions before long.  I've been looking at indexing methods and hoping to get some real simplification done before long.
Ping
Updated to also remove the deprecated methods from SparseSeries, SparseDataFrame.

It looks like iat and at dispatch to _get_value/_set_value.  After this I'd like to do a follow-up to move the contents of _get_value/_set_value to iat/at and get rid of _get_value/_set_value altogether.
can you rebase again just to make sure here
ping
thanks