-
-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
TST/REF: collect tests by method #37449
Conversation
…t-timeseries-2
…t-timeseries-2
…t-timeseries-2
pandas/tests/series/test_npfuncs.py
Outdated
class TestAsArray: | ||
@pytest.mark.parametrize("tz", [None, "US/Central"]) | ||
def test_asarray_object_dt64(self, tz): | ||
ser = Series(date_range("2000", periods=2, tz=tz)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, these are not npfuncs, i think this would be better in a test_datetimes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its for np.asarray
current plan is to get rid of tests.(series|frame).test_datetimes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i get it but these are pure conversion tests should be located with to_numpy() for example and it ufuncs themselves
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is already a dedicated test_ufuncs.py.
The idea behind the collect-by-method approach is for it to be really obvious where a test belongs. to_numpy gets tested in test_to_numpy, .values gets tested in test_values.
np.array is an odd duck because it isnt one of our methods, so this may not be the best long-term place for this (ive been looking at tests.base which has tests shared between Index/Series which this might fit into eventually). how about we revisit this in a few weeks when (hopefully) we're closer to the end of this process
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm i would just move it now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated to move these to tests.base.test_conversion
No description provided.