Skip to content

DataFrame.idxmax returns NaN if all entries are NaN, contradicts idxmax documentation #18206

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

Closed
ghasemnaddaf opened this issue Nov 10, 2017 · 6 comments · Fixed by #18209
Closed
Labels
Docs Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Milestone

Comments

@ghasemnaddaf
Copy link
Contributor

ghasemnaddaf commented Nov 10, 2017

Code Sample, a copy-pastable example if possible

(Pdb) df = pd.DataFrame({'a': [pd.np.nan]*10})
(Pdb) df.idxmax()
a   NaN
dtype: float64
(Pdb) df.idxmax(skipna=True)
a   NaN
dtype: float64
(Pdb) pd.__version__
'0.20.2'

Problem description

According to DataFrame.idxmax documentation: If an entire row/column is NA, the result will be first index

Expected Output

df.idxmax(skipna=True)
a     0

Output of pd.show_versions()


commit: None
python: 3.4.2.final.0
python-bits: 64
OS: Linux
OS-release: 4.9.0-4-amd64
machine: x86_64
processor:
byteorder: little
LC_ALL: None
LANG: en_CA.utf8
LOCALE: en_CA.UTF-8

pandas: 0.20.2
pytest: 2.9.1
pip: 9.0.1
setuptools: 36.2.7
Cython: None
numpy: 1.13.0
scipy: 0.19.0
xarray: None
IPython: 5.1.0
sphinx: None
patsy: None
dateutil: 2.5.3
pytz: 2016.4
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: 2.0.2
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml: None
bs4: None
html5lib: 1.0b10
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: 2.8
s3fs: None
pandas_gbq: None
pandas_datareader: None

@jreback
Copy link
Contributor

jreback commented Nov 10, 2017

the result is correct, its the documentation that is wrong. If you have an all NaN column we have no choice but to return NaN.

@TomAugspurger

@jreback jreback added the Docs label Nov 10, 2017
@ghasemnaddaf
Copy link
Contributor Author

ok thanks @jreback for clarifying, I guessed so since series' idxmax doc. does not mention that clause.
I think I'll manually do .fillna(df.apply(lambda x: x.index[0])) in my code as a workaround

@jreback
Copy link
Contributor

jreback commented Nov 10, 2017

@ghasemnaddaf if you'd like to do a PR to clarify in the docs? (in Series as well) would be great.

@jreback jreback added Difficulty Novice Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Nov 10, 2017
@jreback jreback added this to the Next Major Release milestone Nov 10, 2017
ghasemnaddaf pushed a commit to ghasemnaddaf/pandas that referenced this issue Nov 10, 2017
@ghasemnaddaf
Copy link
Contributor Author

#18209

@jorisvandenbossche
Copy link
Member

Given that you typically use the result of idxmax for indexing, I am not sure the NaN behaviour is necessarily that much better than 0

@jorisvandenbossche
Copy link
Member

Funny thing is that the idxmin docstring correctly says the return value is NA if all values are NA.

Given that you typically use the result of idxmax for indexing, I am not sure the NaN behaviour is necessarily that much better than 0

and given this behaviour has always been like this, I am fine with keeping it like this to be clear. I was just wondering the reason for 0. And you can always use np.armax on the values to get a valid indexer (that one does return 0 for the all NaN case)

@jorisvandenbossche jorisvandenbossche modified the milestones: Next Major Release, 0.21.1 Nov 14, 2017
TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this issue Dec 8, 2017
TomAugspurger pushed a commit that referenced this issue Dec 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants