Skip to content
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

Fix+Test Timedelta.__mul__(nan) #23342

Merged
merged 5 commits into from
Oct 28, 2018
Merged

Conversation

jbrockmendel
Copy link
Member

Specifically, np.float64("NAN") which in master gets caught by the hasattr(other, "dtype") branch and incorrectly returns np.timedelta64("NaT", "ns") instead of NaT

  • closes #xxxx
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@pep8speaks
Copy link

Hello @jbrockmendel! Thanks for submitting the PR.

@codecov
Copy link

codecov bot commented Oct 26, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@c76173b). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #23342   +/-   ##
=========================================
  Coverage          ?   92.22%           
=========================================
  Files             ?      169           
  Lines             ?    51217           
  Branches          ?        0           
=========================================
  Hits              ?    47236           
  Misses            ?     3981           
  Partials          ?        0
Flag Coverage Δ
#multiple 90.66% <ø> (?)
#single 42.23% <ø> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c76173b...6588e19. Read the comment docs.

@@ -328,6 +336,16 @@ def test_td_div_numeric_scalar(self):
assert isinstance(result, Timedelta)
assert result == Timedelta(days=2)

@pytest.mark.parametrize('nan', [np.nan, np.float64('NaN'), float('nan')])
Copy link
Contributor

Choose a reason for hiding this comment

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

nulls_fixture does this

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but it also includes several scalars we dont want to include here.

@@ -277,6 +277,14 @@ def test_td_mul_nat(self, op, td_nat):
with pytest.raises(TypeError):
op(td, td_nat)

@pytest.mark.parametrize('nan', [np.nan, np.float64('NaN'), float('nan')])
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@jreback jreback added Numeric Operations Arithmetic, Comparison, and Logical operations Timedelta Timedelta data type labels Oct 26, 2018
@jreback jreback added this to the 0.24.0 milestone Oct 26, 2018
@jreback jreback merged commit ebd6666 into pandas-dev:master Oct 28, 2018
@jreback
Copy link
Contributor

jreback commented Oct 28, 2018

thanks

@jbrockmendel jbrockmendel deleted the td_mul_nan branch October 28, 2018 05:12
tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Numeric Operations Arithmetic, Comparison, and Logical operations Timedelta Timedelta data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants