Skip doctests in two imported ufuncs by lpsinger · Pull Request #12861 · astropy/astropy

lpsinger · 2022-02-15T19:37:37Z

See #12853, scientific-python/pytest-doctestplus#175

See astropy#12853, scientific-python/pytest-doctestplus#175

pllim · 2022-02-15T19:49:10Z

_ ERROR collecting .../astropy/cosmology/core.py _
.../astropy/cosmology/core.py:398: in __getattr__
    warnings.warn(
E   astropy.utils.exceptions.AstropyDeprecationWarning: `astropy.cosmology.core.__doctest_skip__`
has been moved (since v5.0) and should be imported as ``from astropy.cosmology import __doctest_skip__``.
In future this will raise an exception.

🤯

@nstarman , what is this about?

astropy/astropy/cosmology/core.py

Lines 390 to 407 in cd730f9

    
           def __getattr__(attr): 
        
               from . import flrw 
        
               if hasattr(flrw, attr): 
        
                   import warnings 
        
                   from astropy.utils.exceptions import AstropyDeprecationWarning 
        
                   warnings.warn( 
        
                       f"`astropy.cosmology.core.{attr}` has been moved (since v5.0) and " 
        
                       f"should be imported as ``from astropy.cosmology import {attr}``." 
        
                       " In future this will raise an exception.", 
        
                       AstropyDeprecationWarning 
        
                   ) 
        
                   return getattr(flrw, attr) 
        
               raise AttributeError(f"module {__name__!r} has no attribute {attr!r}.")

nstarman · 2022-02-15T20:07:49Z

@nstarman , what is this about?

A lot of FLRW-related stuff has been moved from astropy.cosmology.core to astropy.cosmology.flrw. However, everything should be imported from the top-level namespace, not the .core nor .flrw sub-modules.

In a few Astropy releases I'll get rid of the __getattr__ and importing stuff in astropy.cosmology.flrw from `astropy.cosmology.core`` will error as expected.

To avoid this raising a warning, a blank __doctest_skip__ = [] could be added to astropy.cosmology.core, then the __getattr__ won't be called.

pllim · 2022-02-15T20:10:31Z

How can adding a __doctest_skip__ here trigger this?

nstarman · 2022-02-15T20:13:52Z

How can adding a __doctest_skip__ here trigger this?

Adding a __doctest_skip__ to astropy.cosmology.flrw means that when using hasattr(astropy.cosmology.core, "__doctest_skip__"), astropy.cosmology.core.__getattr__ will find astropy.cosmology.flrw. __doctest_skip__ and issue the warning. Before it wasn't finding anything.

pllim · 2022-02-15T20:15:11Z

Well, this complicate things...

pllim · 2022-02-15T20:15:54Z

@mhvk , @lpsinger , @nstarman , perhaps easier to move the offending imports to local import and add a note why they were moved?

nstarman · 2022-02-15T20:16:13Z

To avoid this raising a warning, a blank __doctest_skip__ = [] could be added to astropy.cosmology.core, then the __getattr__ won't be called.

e.g. in astropy.cosmology.core

__doctest_skip__ = []  # TODO! remove this when __getattr__ is removed.

lpsinger · 2022-02-15T20:22:12Z

To avoid this raising a warning, a blank __doctest_skip__ = [] could be added to astropy.cosmology.core, then the __getattr__ won't be called.

Ah, there's a precedent for this in

astropy/astropy/cosmology/core.py

Line 27 in cd730f9

__doctest_requires__ = {} # needed until __getattr__ removed

.

pllim · 2022-02-15T20:44:11Z

I cherry-picked the commits here onto #12853 . We will see... 🤞

lpsinger · 2022-02-15T21:24:18Z

Please also cherry-pick 3f5797e.

pllim · 2022-02-15T21:32:30Z

astropy/cosmology/tests/test_flrw.py

 from .test_core import CosmologySubclassTest as CosmologyTest
 from .test_core import FlatCosmologyMixinTest, ParameterTestMixin, invalid_zs, valid_zs

+__doctest_skip__ = ['ellipkinc', 'hyp2f1']


Test file too? I don't know how I feel about this. I feel like we are abusing __doctest_skip__.

Well, there is nothing about a ufunc that you can find out by introspection to determine the module it was declared in.

I get that a ufunc can't be introspected, but how does pytest-doctestplus pick up the test file in the first place? Shouldn't they skipped because they aren't doctested?

Those two ufuncs do have doctests in their docstrings.

Sorry for the confusion, I meant I thought pytest-doctestplus filtered out test files ab initio, but I guess that's not the case?

Come again? I'm sorry, I didn't understand your question.

I presumed when pytest-doctestplus was assembling the list of docstrings it needed to check, it would skip over test files. E.g.

module/ file.py tests/ # skipped test_file.py # skipped, so it doesn't matter if there's a ufunc inside

Oh, that's probably a good idea. But I doubt that even pytest does that by default, does it?

pllim · 2022-02-23T14:05:40Z

@lpsinger , do we still need this in light of recent development over at scientific-python/pytest-doctestplus#175 ?

lpsinger · 2022-02-23T14:46:58Z

@lpsinger , do we still need this in light of recent development over at astropy/pytest-doctestplus#175 ?

No.

lpsinger · 2022-02-24T17:30:36Z

Shall we close this one, then?

pllim · 2022-02-24T17:37:46Z

Sure. Thanks!

Skip doctests in two imported ufuncs

4d0e62f

See astropy#12853, scientific-python/pytest-doctestplus#175

lpsinger mentioned this pull request Feb 15, 2022

EXP: Run test suite with dev pytest-doctestplus #12853

Closed

10 tasks

github-actions bot added the cosmology label Feb 15, 2022

Add __doctest_skip__

4405de2

pllim added this to the v5.0.2 milestone Feb 15, 2022

pllim added Docs no-changelog-entry-needed testing backport-v5.0.x labels Feb 15, 2022

Add another skip

3f5797e

pllim reviewed Feb 15, 2022

View reviewed changes

pllim mentioned this pull request Feb 15, 2022

Doctests in ufuncs should respect __doctest_skip__ scientific-python/pytest-doctestplus#175

Merged

pllim closed this Feb 24, 2022

lpsinger deleted the doctest-skip-flrw branch February 24, 2022 17:39

Uh oh!

Conversation

lpsinger commented Feb 15, 2022

Uh oh!

pllim commented Feb 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nstarman commented Feb 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pllim commented Feb 15, 2022

Uh oh!

nstarman commented Feb 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pllim commented Feb 15, 2022

Uh oh!

pllim commented Feb 15, 2022

Uh oh!

nstarman commented Feb 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lpsinger commented Feb 15, 2022

Uh oh!

pllim commented Feb 15, 2022

Uh oh!

lpsinger commented Feb 15, 2022

Uh oh!

pllim Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

lpsinger Feb 16, 2022

Choose a reason for hiding this comment

Uh oh!

nstarman Feb 16, 2022

Choose a reason for hiding this comment

Uh oh!

lpsinger Feb 16, 2022

Choose a reason for hiding this comment

Uh oh!

nstarman Feb 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lpsinger Feb 16, 2022

Choose a reason for hiding this comment

Uh oh!

nstarman Feb 16, 2022

Choose a reason for hiding this comment

Uh oh!

lpsinger Feb 16, 2022

Choose a reason for hiding this comment

Uh oh!

pllim commented Feb 23, 2022

Uh oh!

lpsinger commented Feb 23, 2022

Uh oh!

lpsinger commented Feb 24, 2022

Uh oh!

pllim commented Feb 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pllim commented Feb 15, 2022 •

edited

Loading

nstarman commented Feb 15, 2022 •

edited

Loading

nstarman commented Feb 15, 2022 •

edited

Loading

nstarman commented Feb 15, 2022 •

edited

Loading

nstarman Feb 16, 2022 •

edited

Loading