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

BUG: fix construction from read-only non-ns datetime64 numpy array #34844

Merged

Conversation

jorisvandenbossche
Copy link
Member

Closes #34843

@@ -843,6 +843,9 @@ Datetimelike
- Bug in :meth:`DatetimeIndex.intersection` and :meth:`TimedeltaIndex.intersection` with results not having the correct ``name`` attribute (:issue:`33904`)
- Bug in :meth:`DatetimeArray.__setitem__`, :meth:`TimedeltaArray.__setitem__`, :meth:`PeriodArray.__setitem__` incorrectly allowing values with ``int64`` dtype to be silently cast (:issue:`33717`)
- Bug in subtracting :class:`TimedeltaIndex` from :class:`Period` incorrectly raising ``TypeError`` in some cases where it should succeed and ``IncompatibleFrequency`` in some cases where it should raise ``TypeError`` (:issue:`33883`)
- Bug in constructing a Series or Index from a read-only numpy array with non-ns
Copy link
Member

Choose a reason for hiding this comment

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

:class: for Series and Index? numpy -> NumPy?

@@ -145,3 +158,14 @@ def test_constructor_datetime_outofbound(self, a, klass):
msg = "Out of bounds"
with pytest.raises(pd.errors.OutOfBoundsDatetime, match=msg):
klass(a, dtype="datetime64[ns]")

def test_constructor_datetime_nonns(self, constructor):
Copy link
Member

Choose a reason for hiding this comment

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

add the issue number

Copy link
Member Author

Choose a reason for hiding this comment

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

See a few lines below

@jorisvandenbossche
Copy link
Member Author

Travis failure are the numpy dev warnings

@jorisvandenbossche jorisvandenbossche merged commit a8bbe75 into pandas-dev:master Jun 18, 2020
@jorisvandenbossche jorisvandenbossche deleted the non-ns-non-writable branch June 18, 2020 06:47
@@ -13,6 +13,19 @@
from pandas.core.base import NoNewAttributesMixin, PandasObject


@pytest.fixture(
Copy link
Contributor

Choose a reason for hiding this comment

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

@jorisvandenbossche if you hadn’t merged this so quickly i would have objected here

creating this fixture for a single use is not that great

we typically also call this klass and don’t much care about dataframe-dict here

@jorisvandenbossche
Copy link
Member Author

we typically also call this klass

There are several existing occurrences of a "constructor" fixture, already

and don’t much care about dataframe-dict here

We actually do, as the test above has an xfail because DataFrame(dict) vs DataFrame(array) behaves differently (the sanitization of datetime64 values follows a different code path in both cases).
In this case both work, but so I think it is good to test both.

creating this fixture for a single use is not that great

Yeah, that is because I first wanted to share it with the other existing test. But because that has an xfail on one of the fixture parameters, making it difficult to share. Once that bug is fixed, it can be shared though.

@jorisvandenbossche
Copy link
Member Author

as the test above has an xfail because DataFrame(dict) vs DataFrame(array) behaves differently (the sanitization of datetime64 values follows a different code path in both cases).

Which is #26919

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Datetime Datetime data dtype
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Series(..) coerces datetime64[non-ns] array differently depending on writable flag
4 participants