Skip to content

TST: doctest compat with pandas 1.1.0 - #10609

Merged
pllim merged 1 commit into
astropy:masterfrom
pllim:pandas-doctest
Jul 30, 2020
Merged

TST: doctest compat with pandas 1.1.0#10609
pllim merged 1 commit into
astropy:masterfrom
pllim:pandas-doctest

Conversation

@pllim

@pllim pllim commented Jul 28, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Description

This pull request is to address doctest failures with pandas 1.1.0. Given we don't test for older pandas, should be okay? 🤞

Fixes #10608

p.s. Milestoned to LTS so the CI there would pass too, but feel free to re-milestone if that is not applicable.

@astropy-bot astropy-bot Bot added the table label Jul 28, 2020
@pllim pllim added this to the v4.0.2 milestone Jul 28, 2020
@pllim
pllim requested review from mhvk and taldcroft July 28, 2020 20:18
@pllim

This comment has been minimized.

Sign in to view
@mhvk

mhvk commented Jul 29, 2020

Copy link
Copy Markdown
Contributor

I'll have to leave this to @taldcroft, but it would seem that it might be best to see if there is a way to remove the 0 days part somehow...

This was referenced Jul 29, 2020
Closed
Closed
@pllim

pllim commented Jul 29, 2020

Copy link
Copy Markdown
Member Author

I asked at pandas-dev/pandas#35463 . 🤞

@pllim pllim mentioned this pull request Jul 30, 2020
Closed
@pllim

pllim commented Jul 30, 2020

Copy link
Copy Markdown
Member Author

pandas devs say this change is permanent. There is nothing we can do about it.

mhvk
mhvk reviewed Jul 30, 2020
View reviewed changes

@mhvk mhvk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The change looks good but I think we have, sadly, to make it version dependent - no reason for people to have failing tests if they have an older version of pandas.

@pllim

This comment has been minimized.

Sign in to view
@mhvk

mhvk commented Jul 30, 2020

Copy link
Copy Markdown
Contributor

Actually, maybe for this particular test, we can just xfail for older versions of pandas?

@pllim

This comment has been minimized.

Sign in to view
@mhvk

mhvk commented Jul 30, 2020

Copy link
Copy Markdown
Contributor

I should look better before commenting...

But given things like .. doctest-requires:: numpy>=1.17 in the rst files, I think that adjusting

__doctest_requires__ = {'*pandas': ['pandas']}

at the top of table.py to 'pandas>=1.1' should do the trick...

@pllim
TST: doctest compat with pandas 1.1.0
cba5792 
Pin pandas in doctest, suggested by @mhvk
@pllim

pllim commented Jul 30, 2020

Copy link
Copy Markdown
Member Author

@mhvk , tests passed, except that one failure in dev job that isn't related.

mhvk
mhvk approved these changes Jul 30, 2020
View reviewed changes

@mhvk mhvk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK, changes look good to me. Did you check that with an older pandas, tests still passed (i.e., that this doctest is indeed skipped)? Approving assuming that. THanks!

@pllim

pllim commented Jul 30, 2020

Copy link
Copy Markdown
Member Author

This is what I get locally. Can't tell if it is skipped or not, but nothing failed. Shall we merge then?

============================= test session starts ==============================
platform linux -- Python 3.8.3, pytest-5.4.3, py-1.9.0, pluggy-0.13.1

Running tests with Astropy version 4.2.dev449+g28d3cf175.
Running tests in astropy/table/table.py.

Date: 2020-07-30T17:53:43

Platform: Linux-3.10.0-1127.13.1.el7.x86_64-x86_64-with-glibc2.10

Executable: .../miniconda/envs/py38/bin/python

Full Python Version: 
3.8.3 (default, May 19 2020, 18:47:26) 
[GCC 7.3.0]

encodings: sys: utf-8, locale: UTF-8, filesystem: utf-8
byteorder: little
float info: dig: 15, mant_dig: 15

Package versions: 
Numpy: 1.19.0
Scipy: 1.5.1
Matplotlib: 3.2.2
h5py: 2.10.0
Pandas: 1.0.5
PyERFA: 1.7.0
Cython: not available
Scikit-image: 0.17.2
asdf: 2.6.0

Using Astropy options: remote_data: none.

rootdir: ..., inifile: setup.cfg
plugins: hypothesis-5.16.1, openfiles-0.5.0, remotedata-0.3.1, arraydiff-0.3, asdf-2.6.0, astropy-header-0.1.2, doctestplus-0.8.0.dev0
collected 20 items                                                             

astropy/table/table.py ....................                              [100%]

============================== 20 passed in 0.69s ==============================

@pllim

pllim commented Jul 30, 2020

Copy link
Copy Markdown
Member Author

For the sake of not blocking PRs any longer, I am merging this. Thanks for the review, @mhvk !

@pllim
pllim merged commit d9ad8d4 into astropy:master Jul 30, 2020
@pllim
pllim deleted the pandas-doctest branch July 30, 2020 22:49
@mhvk

mhvk commented Jul 30, 2020

Copy link
Copy Markdown
Contributor

Yes, that did do it, the travis run has 22 .
Apologies for sending us up several wrong trees!

@pllim

pllim commented Jul 31, 2020

Copy link
Copy Markdown
Member Author

Oh, I didn't compare the numbers; duh! 🤦

No worries, @mhvk ! There were all good ideas.

eteq pushed a commit that referenced this pull request Aug 3, 2020
@pllim @eteq
Merge pull request #10609 from pllim/pandas-doctest
f941a91 
TST: doctest compat with pandas 1.1.0
@pllim pllim mentioned this pull request Aug 27, 2020
Merged
astrofrog pushed a commit that referenced this pull request Oct 6, 2020
@pllim @astrofrog
Merge pull request #10609 from pllim/pandas-doctest
d2f5c53 
TST: doctest compat with pandas 1.1.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhvk mhvk mhvk approved these changes
@taldcroft taldcroft Awaiting requested review from taldcroft

Assignees

No one assigned

Labels

Docs no-changelog-entry-needed table testing

Projects

None yet

Milestone

v4.0.2

Development

Successfully merging this pull request may close these issues.

TST: Table doctest failures with pandas 1.1.0

2 participants

@pllim @mhvk