-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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: split up test_concat.py #37243 - more follows up #37387
Conversation
* created test_categorical.py
* created test_empty.py * changes to test_series.py
I will finish it tomorrow, i didn't have much free time today. There is not much left, i have to create test_sort and test_invalid and make some small changes. |
* created test_categorical.py
* created test_empty.py * changes to test_series.py
* created test_invalid.py * created test_sort.py
…/pandas into i37243_more_follows_up
* created test_index.py * small changes to test_empty.py, test_datetimes.py, test_dataframe.py
most of the work should be done, all tests are working, it was 291 passed and 1 xfailed before and it's the same now. |
I haven't touched test_append_common and test_append, but these files looks ok for me. |
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.
can you show the before / after tests counts (they should match).
make the change in conftest.py and i think this should be able to be green if you merge master.
import pandas._testing as tm | ||
|
||
|
||
@pytest.fixture(params=[True, False]) |
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 would move fixtures to a concat/conftest.py (unless they are truly specific to a particular file)
import pandas._testing as tm | ||
|
||
|
||
@pytest.fixture(params=[True, False]) |
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.
yeah so this is for sure a common fixture
def test_concat_series_axis1(self, sort=sort): | ||
def test_concat_series_axis1(self, sort=True): | ||
ts = tm.makeTimeSeries() | ||
|
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 don't know about this, after moving sort to conftest.py, this test was giving me error, so i have changed default to True. Is this good, or do i have to move fixture to this file and revert this change?
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.
pls revert this, you should not be changing any code (literally just removing the fixture definition itself)
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.
ok, changed.
* reverted changes to test_series.py
|
||
|
||
def test_concat_timedelta64_block(): | ||
from pandas import to_timedelta |
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.
import at the top
df = DataFrame({"time": rng}) | ||
|
||
result = concat([df, df]) | ||
assert (result.iloc[:10]["time"] == rng).all() |
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.
ca n you this use tm.assert_frame_equal instead (and use a fully constructed dataframe)
couple of comments, ping on green. |
ok, i will check it in a moment. |
* small changes in test_datetimes.py
@jreback everything done |
Can I ask what does "ping on green" mean? Is it the green "review changes" button in "files changed" section? Or is it just typing comment, so you can see new activity? |
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.
green
thanks @laffite56 |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff