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

Doctest failure depending on terminal width #16307

Open
astrofrog opened this issue Apr 18, 2024 · 13 comments
Open

Doctest failure depending on terminal width #16307

astrofrog opened this issue Apr 18, 2024 · 13 comments

Comments

@astrofrog
Copy link
Member

If I run the test suite in a large terminal I get a doctest failure:

___________________________________________________ [doctest] io.rst ____________________________________________________
104 
105     >>> from astropy import units as u
106     >>> from astropy.timeseries import BinnedTimeSeries
107     >>> binned_filename = get_pkg_data_filename('data/binned.csv',
108     ...                                          package='astropy.timeseries.tests')
109     >>> ts = BinnedTimeSeries.read(binned_filename, format='ascii.csv',
110     ...                            time_bin_start_column='time_start',
111     ...                            time_bin_size_column='bin_size',
112     ...                            time_bin_size_unit=u.s)
113     >>> ts[:3]  # doctest: +ELLIPSIS
Differences (unified diff with -expected +actual):
    @@ -1,8 +1,8 @@
     <BinnedTimeSeries length=3>
    -     time_bin_start     time_bin_size ...    E       F
    -                              s       ...
    -          Time             float64    ... float64 float64
    ------------------------ ------------- ... ------- -------
    -2016-03-22T12:30:31.000           3.0 ...   28.87   63.44
    -2016-03-22T12:30:34.000           3.0 ...   27.76   59.98
    -2016-03-22T12:30:37.000           3.0 ...   27.04   59.61
    +     time_bin_start     time_bin_size         time_end           A       B       C       D       E       F   
    +                              s                                                                              
    +          Time             float64             str23          float64 float64 float64 float64 float64 float64
    +----------------------- ------------- ----------------------- ------- ------- ------- ------- ------- -------
    +2016-03-22T12:30:31.000           3.0 2016-03-22T12:30:34.000  164.93  114.73   26.27   19.21   28.87   63.44
    +2016-03-22T12:30:34.000           3.0 2016-03-22T12:30:37.000  164.89  114.75   26.22   19.07   27.76   59.98
    +2016-03-22T12:30:37.000           3.0 2016-03-22T12:30:40.000  164.63  115.04   25.78   19.01   27.04   59.61

/home/tom/Code/astropy/docs/timeseries/io.rst:113: DocTestFailure

@taldcroft - as you must have encountered this in the table docs before, do you know of a way to prevent this?

@taldcroft
Copy link
Member

@astrofrog - I don't recall encountering that in doc tests, likely because I usually use example tables that are short and narrow.

I notice you added # doctest: +ELLIPSIS, which in a perfect world would have done the trick here. Barring that, I'd suggest using t[:3].pprint_all() to force printing the entire table. This is basically the strategy in unit tests to ensure consistency.

@astrofrog
Copy link
Member Author

@taldcroft - ah yes table suffers from this too, if you make your terminal e.g. 40 characters wide, some of the doctests fail. We should probably figure out a way to make all these more robust!

@neutrinoceros
Copy link
Contributor

I vaguely recall seeing this issue a couple month back, but I can't find the duplicate, so maybe it was never reported. However, it seems related to #15828

@taldcroft
Copy link
Member

Is there a way in doctest to have some invisible Python setup that always gets run? We could configure the terminal to be big in that case.

@neutrinoceros
Copy link
Contributor

I couldn't find anything of the sort in https://docs.python.org/3/library/doctest.html or pytest-doctest-plus's documentation, so I think the answer is no. Additionally, it would probably make all doctests slightly slower and slightly harder to debug so I'm not convinced it's a desirable approach.

I notice you added # doctest: +ELLIPSIS, which in a perfect world would have done the trick here.

I actually can't understand why it does not do the trick here. Does anyone know ?

@pllim
Copy link
Member

pllim commented Apr 18, 2024

Maybe try removing the leading whitespace before ...? 🤷‍♀️

Or maybe Tom R should use a smaller terminal. I heard 80-char width is pretty neat. 😉

@pllim
Copy link
Member

pllim commented Apr 18, 2024

invisible Python setup that always gets run

Yes, https://www.sphinx-doc.org/en/master/usage/extensions/doctest.html#directives (e.g., .. testsetup::).

@taldcroft
Copy link
Member

Yes, https://www.sphinx-doc.org/en/master/usage/extensions/doctest.html#directives (e.g., .. testsetup::).

That looks promising for exactly this situation (consistent output of a table that can depend on terminal size).

@astrofrog
Copy link
Member Author

Maybe we could just set things globally in conftest.py?

@taldcroft
Copy link
Member

Right, if a global conftest.py is actually used in doc tests then I think that would be a fine option. We could just set conf.max_width and conf.max_lines both to -1 implying no limits. Then see if any existing tests break, but hopefully not many.

@pllim
Copy link
Member

pllim commented Apr 18, 2024

Maybe here:

def pytest_configure(config):

But maybe also a good practice to unset it after testing is done?

def pytest_unconfigure(config):

@saimn
Copy link
Contributor

saimn commented Apr 22, 2024

Reminds me #8691 :)

@pllim
Copy link
Member

pllim commented Apr 22, 2024

Ah, so this and #8691 are the same problem? Should we close one of them as duplicate? Which one?

@pllim pllim added the Docs label Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants