Skip to content

CLN: address FIXME/TODO/XXX comments #33858

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

Merged
merged 3 commits into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ci/setup_env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ conda list pandas
echo "[Build extensions]"
python setup.py build_ext -q -i -j2

# XXX: Some of our environments end up with old versions of pip (10.x)
# TODO: Some of our environments end up with old versions of pip (10.x)
# Adding a new enough version of pip to the requirements explodes the
# solve time. Just using pip to update itself.
# - py35_macos
Expand Down
4 changes: 2 additions & 2 deletions pandas/_libs/src/parser/tokenizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ See LICENSE for the license
#define PARSER_OUT_OF_MEMORY -1

/*
* XXX Might want to couple count_rows() with read_rows() to avoid duplication
* of some file I/O.
* TODO: Might want to couple count_rows() with read_rows() to avoid
* duplication of some file I/O.
*/

typedef enum {
Expand Down
2 changes: 1 addition & 1 deletion pandas/_libs/tslibs/strptime.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ class TimeRE(dict):
'w': r"(?P<w>[0-6])",
# W is set below by using 'U'
'y': r"(?P<y>\d\d)",
# XXX: Does 'Y' need to worry about having less or more than
# TODO: Does 'Y' need to worry about having less or more than
# 4 digits?
'Y': r"(?P<Y>\d\d\d\d)",
'z': r"(?P<z>[+-]\d\d:?[0-5]\d(:?[0-5]\d(\.\d{1,6})?)?|Z)",
Expand Down
1 change: 1 addition & 0 deletions pandas/core/algorithms.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ def _ensure_data(values, dtype=None):
if values.ndim > 1 and is_datetime64_ns_dtype(values):
# Avoid calling the DatetimeIndex constructor as it is 1D only
# Note: this is reached by DataFrame.rank calls GH#27027
# TODO(EA2D): special case not needed with 2D EAs
asi8 = values.view("i8")
dtype = values.dtype
return asi8, dtype
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/arrays/sparse/array.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ def __init__(
# TODO: disentangle the fill_value dtype inference from
# dtype inference
if data is None:
# XXX: What should the empty dtype be? Object or float?
# TODO: What should the empty dtype be? Object or float?
data = np.array([], dtype=dtype)

if not is_array_like(data):
Expand Down
1 change: 1 addition & 0 deletions pandas/core/dtypes/concat.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ def concat_categorical(to_concat, axis: int = 0):
]
result = concat_compat(to_concat)
if axis == 1:
# TODO(EA2D): not necessary with 2D EAs
result = result.reshape(1, len(result))
return result

Expand Down
1 change: 1 addition & 0 deletions pandas/core/groupby/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,7 @@ def _cython_agg_blocks(
# e.g. block.values was an IntegerArray
# (1, N) case can occur if block.values was Categorical
# and result is ndarray[object]
# TODO(EA2D): special casing not needed with 2D EAs
assert result.ndim == 1 or result.shape[0] == 1
try:
# Cast back if feasible
Expand Down
6 changes: 3 additions & 3 deletions pandas/core/groupby/ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,8 +476,8 @@ def _cython_operation(

if is_datetime64tz_dtype(values.dtype):
# Cast to naive; we'll cast back at the end of the function
# TODO: possible need to reshape? kludge can be avoided when
# 2D EA is allowed.
# TODO: possible need to reshape?
# TODO(EA2D):kludge can be avoided when 2D EA is allowed.
values = values.view("M8[ns]")

is_datetimelike = needs_i8_conversion(values.dtype)
Expand Down Expand Up @@ -717,7 +717,7 @@ def _aggregate_series_pure_python(
if isinstance(res, (Series, Index, np.ndarray)):
if len(res) == 1:
# e.g. test_agg_lambda_with_timezone lambda e: e.head(1)
# FIXME: are we potentially losing import res.index info?
# FIXME: are we potentially losing important res.index info?
res = res.item()
else:
raise ValueError("Function does not reduce")
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/internals/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -1721,7 +1721,7 @@ def take_nd(
return self.make_block_same_class(new_values, new_mgr_locs)

def _can_hold_element(self, element: Any) -> bool:
# XXX: We may need to think about pushing this onto the array.
# TODO: We may need to think about pushing this onto the array.
# We're doing the same as CategoricalBlock here.
return True

Expand Down
1 change: 1 addition & 0 deletions pandas/core/internals/concat.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ def is_na(self):
elif is_sparse(self.block.values.dtype):
return False
elif self.block.is_extension:
# TODO(EA2D): no need for special case with 2D EAs
values_flat = values
else:
values_flat = values.ravel(order="K")
Expand Down
2 changes: 1 addition & 1 deletion pandas/io/parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1851,7 +1851,7 @@ def _cast_types(self, values, cast_type, column):
)

if not is_object_dtype(values) and not known_cats:
# XXX this is for consistency with
# TODO: this is for consistency with
# c-parser which parses all categories
# as strings
values = astype_nansafe(values, str)
Expand Down
12 changes: 4 additions & 8 deletions pandas/tests/arithmetic/test_datetime64.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,10 @@ def test_dt64arr_nat_comparison(self, tz_naive_fixture, box_with_array):
ts = pd.Timestamp.now(tz)
ser = pd.Series([ts, pd.NaT])

# FIXME: Can't transpose because that loses the tz dtype on
# the NaT column
obj = tm.box_expected(ser, box, transpose=False)
obj = tm.box_expected(ser, box)

expected = pd.Series([True, False], dtype=np.bool_)
expected = tm.box_expected(expected, xbox, transpose=False)
expected = tm.box_expected(expected, xbox)

result = obj == ts
tm.assert_equal(result, expected)
Expand Down Expand Up @@ -843,10 +841,8 @@ def test_dt64arr_add_sub_td64_nat(self, box_with_array, tz_naive_fixture):
other = np.timedelta64("NaT")
expected = pd.DatetimeIndex(["NaT"] * 9, tz=tz)

# FIXME: fails with transpose=True due to tz-aware DataFrame
# transpose bug
obj = tm.box_expected(dti, box_with_array, transpose=False)
expected = tm.box_expected(expected, box_with_array, transpose=False)
obj = tm.box_expected(dti, box_with_array)
expected = tm.box_expected(expected, box_with_array)

result = obj + other
tm.assert_equal(result, expected)
Expand Down
5 changes: 2 additions & 3 deletions pandas/tests/arithmetic/test_period.py
Original file line number Diff line number Diff line change
Expand Up @@ -906,9 +906,8 @@ def test_pi_add_offset_n_gt1_not_divisible(self, box_with_array):
pi = pd.PeriodIndex(["2016-01"], freq="2M")
expected = pd.PeriodIndex(["2016-04"], freq="2M")

# FIXME: with transposing these tests fail
pi = tm.box_expected(pi, box_with_array, transpose=False)
expected = tm.box_expected(expected, box_with_array, transpose=False)
pi = tm.box_expected(pi, box_with_array)
expected = tm.box_expected(expected, box_with_array)

result = pi + to_offset("3M")
tm.assert_equal(result, expected)
Expand Down
1 change: 1 addition & 0 deletions pandas/tests/arrays/boolean/test_logical.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def test_empty_ok(self, all_logical_operators):
result = getattr(a, op_name)(False)
tm.assert_extension_array_equal(a, result)

# FIXME: dont leave commented-out
# TODO: pd.NA
# result = getattr(a, op_name)(pd.NA)
# tm.assert_extension_array_equal(a, result)
Expand Down
4 changes: 0 additions & 4 deletions pandas/tests/base/test_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ class TestToIterable:
],
ids=["tolist", "to_list", "list", "iter"],
)
@pytest.mark.filterwarnings("ignore:\\n Passing:FutureWarning")
# TODO(GH-24559): Remove the filterwarnings
def test_iterable(self, index_or_series, method, dtype, rdtype):
# gh-10904
# gh-13258
Expand Down Expand Up @@ -104,8 +102,6 @@ def test_iterable_items(self, dtype, rdtype):
@pytest.mark.parametrize(
"dtype, rdtype", dtypes + [("object", int), ("category", int)]
)
@pytest.mark.filterwarnings("ignore:\\n Passing:FutureWarning")
# TODO(GH-24559): Remove the filterwarnings
def test_iterable_map(self, index_or_series, dtype, rdtype):
# gh-13236
# coerce iteration to underlying python / pandas types
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/extension/base/dtype.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def test_check_dtype(self, data):
else:
expected = pd.Series([True, True, False, False], index=list("ABCD"))

# XXX: This should probably be *fixed* not ignored.
# FIXME: This should probably be *fixed* not ignored.
# See libops.scalar_compare
with warnings.catch_warnings():
warnings.simplefilter("ignore", DeprecationWarning)
Expand Down
3 changes: 2 additions & 1 deletion pandas/tests/extension/base/getitem.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ def test_getitem_integer_with_missing_raises(self, data, idx):
with pytest.raises(ValueError, match=msg):
data[idx]

# TODO this raises KeyError about labels not found (it tries label-based)
# FIXME: dont leave commented-out
# TODO: this raises KeyError about labels not found (it tries label-based)
# import pandas._testing as tm
# s = pd.Series(data, index=[tm.rands(4) for _ in range(len(data))])
# with pytest.raises(ValueError, match=msg):
Expand Down
1 change: 1 addition & 0 deletions pandas/tests/extension/test_boolean.py
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ class TestUnaryOps(base.BaseUnaryOpsTests):
pass


# FIXME: dont leave commented-out
# TODO parsing not yet supported
# class TestParsing(base.BaseParsingTests):
# pass
5 changes: 0 additions & 5 deletions pandas/tests/frame/methods/test_round.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,6 @@ def test_round(self):
# nan in Series round
nan_round_Series = Series({"col1": np.nan, "col2": 1})

# TODO(wesm): unused?
expected_nan_round = DataFrame( # noqa
{"col1": [1.123, 2.123, 3.123], "col2": [1.2, 2.2, 3.2]}
)

msg = "integer argument expected, got float"
with pytest.raises(TypeError, match=msg):
df.round(nan_round_Series)
Expand Down
3 changes: 1 addition & 2 deletions pandas/tests/indexes/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -561,8 +561,7 @@ def test_equals(self, indices):
assert not indices.equals(np.array(indices))

# Cannot pass in non-int64 dtype to RangeIndex
if not isinstance(indices, (RangeIndex, CategoricalIndex)):
# TODO: CategoricalIndex can be re-allowed following GH#32167
if not isinstance(indices, RangeIndex):
same_values = Index(indices, dtype=object)
assert indices.equals(same_values)
assert same_values.equals(indices)
Expand Down
4 changes: 2 additions & 2 deletions pandas/tests/indexes/datetimes/test_date_range.py
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ def test_daterange_bug_456(self):
# GH #456
rng1 = bdate_range("12/5/2011", "12/5/2011")
rng2 = bdate_range("12/2/2011", "12/5/2011")
rng2._data.freq = BDay() # TODO: shouldn't this already be set?
assert rng2._data.freq == BDay()

result = rng1.union(rng2)
assert isinstance(result, DatetimeIndex)
Expand Down Expand Up @@ -905,7 +905,7 @@ def test_daterange_bug_456(self):
# GH #456
rng1 = bdate_range("12/5/2011", "12/5/2011", freq="C")
rng2 = bdate_range("12/2/2011", "12/5/2011", freq="C")
rng2._data.freq = CDay() # TODO: shouldn't this already be set?
assert rng2._data.freq == CDay()

result = rng1.union(rng2)
assert isinstance(result, DatetimeIndex)
Expand Down
3 changes: 0 additions & 3 deletions pandas/tests/indexing/test_chaining_and_caching.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,6 @@ def test_cache_updating(self):
df["f"] = 0
df.f.values[3] = 1

# TODO(wesm): unused?
# y = df.iloc[np.arange(2, len(df))]

df.f.values[3] = 2
expected = DataFrame(
np.zeros((5, 6), dtype="int64"),
Expand Down
8 changes: 4 additions & 4 deletions pandas/tests/io/excel/test_style.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
)
def test_styler_to_excel(engine):
def style(df):
# XXX: RGB colors not supported in xlwt
# TODO: RGB colors not supported in xlwt
return DataFrame(
[
["font-weight: bold", "", ""],
Expand All @@ -47,7 +47,7 @@ def assert_equal_style(cell1, cell2, engine):
pytest.xfail(
reason=(f"GH25351: failing on some attribute comparisons in {engine}")
)
# XXX: should find a better way to check equality
# TODO: should find a better way to check equality
assert cell1.alignment.__dict__ == cell2.alignment.__dict__
assert cell1.border.__dict__ == cell2.border.__dict__
assert cell1.fill.__dict__ == cell2.fill.__dict__
Expand Down Expand Up @@ -98,15 +98,15 @@ def custom_converter(css):

# (2) check styling with default converter

# XXX: openpyxl (as at 2.4) prefixes colors with 00, xlsxwriter with FF
# TODO: openpyxl (as at 2.4) prefixes colors with 00, xlsxwriter with FF
alpha = "00" if engine == "openpyxl" else "FF"

n_cells = 0
for col1, col2 in zip(wb["frame"].columns, wb["styled"].columns):
assert len(col1) == len(col2)
for cell1, cell2 in zip(col1, col2):
ref = f"{cell2.column}{cell2.row:d}"
# XXX: this isn't as strong a test as ideal; we should
# TODO: this isn't as strong a test as ideal; we should
# confirm that differences are exclusive
if ref == "B2":
assert not cell1.font.bold
Expand Down
2 changes: 0 additions & 2 deletions pandas/tests/io/formats/test_css.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ def test_css_parse_invalid(invalid_css, remainder):
with tm.assert_produces_warning(CSSWarning):
assert_same_resolution(invalid_css, remainder)

# TODO: we should be checking that in other cases no warnings are raised


@pytest.mark.parametrize(
"shorthand,expansions",
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/io/test_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -1130,7 +1130,7 @@ def setup_method(self, load_iris_data):
self.conn.close()
self.conn = self.__engine
self.pandasSQL = sql.SQLDatabase(self.__engine)
# XXX:
# FIXME: dont leave commented-out
# super().teardown_method(method)


Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/reshape/test_concat.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ def test_concatlike_dtypes_coercion(self):
# same dtype is tested in test_concatlike_same_dtypes
continue
elif typ1 == "category" or typ2 == "category":
# ToDo: suspicious
# TODO: suspicious
continue

# specify expected dtype
Expand Down
1 change: 0 additions & 1 deletion pandas/tests/scalar/timedelta/test_arithmetic.py
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,6 @@ def test_td_rfloordiv_numeric_series(self):
msg = "Invalid dtype"
with pytest.raises(TypeError, match=msg):
# Deprecated GH#19761, enforced GH#29797
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could probably remove this comment as well or does it adds value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either way i guess, the historical reference doesnt hurt

# TODO: GH-19761. Change to TypeError.
ser // td

# ----------------------------------------------------------------
Expand Down
2 changes: 0 additions & 2 deletions pandas/tests/series/indexing/test_datetime.py
Original file line number Diff line number Diff line change
Expand Up @@ -556,8 +556,6 @@ def test_indexing_unordered():
ts2 = pd.concat([ts[0:4], ts[-4:], ts[4:-4]])

for t in ts.index:
# TODO: unused?
s = str(t) # noqa

expected = ts[t]
result = ts2[t]
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/series/indexing/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ def test_setitem_with_tz(tz):


def test_setitem_with_tz_dst():
# GH XXX
# GH XXX TODO: fill in GH ref
tz = "US/Eastern"
orig = pd.Series(pd.date_range("2016-11-06", freq="H", periods=3, tz=tz))
assert orig.dtype == f"datetime64[ns, {tz}]"
Expand Down
2 changes: 1 addition & 1 deletion pandas/tseries/offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ def rollforward(self, dt):
def is_on_offset(self, dt):
if self.normalize and not _is_normalized(dt):
return False
# XXX, see #1395
# TODO, see #1395
if type(self) == DateOffset or isinstance(self, Tick):
return True

Expand Down