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

maint(fix): Add deprecation warning for accidental imports #19316

Merged
merged 3 commits into from May 18, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
168 changes: 168 additions & 0 deletions sympy/__init__.py
Expand Up @@ -496,3 +496,171 @@ def __sympy_debug():
# sympy.deprecated:
'C', 'ClassRegistry', 'class_registry',
]


#=======================================================================
#
# XXX: The names below were importable before sympy 1.6 using
#
# from sympy import *
#
# This happened implicitly because there was no __all__ defined in this
# __init__.py file. Not every package is imported. The list matches what
# would have been imported before.
#
#=======================================================================


__all__.extend([
'algebras',
'assumptions',
'calculus',
'codegen',
'combinatorics',
'concrete',
'deprecated',
'discrete',
'external',
'functions',
'geometry',
'interactive',
'multipledispatch',
'ntheory',
'parsing',
# 'physics',
Copy link
Member

Choose a reason for hiding this comment

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

Maybe remove if this wasn't imported before?

'plotting',
'polys',
'printing',
'release',
'strategies',
'tensor',
'utilities',
])


#=======================================================================
#
# XXX: The names below were importable before sympy 1.6 using
#
# from sympy import *
#
# This happened implicitly because there was no __all__ defined in this
# __init__.py file. The plan is to remove them but for now they remain
# importable but will give a deprecation warning when used.
#
#=======================================================================
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the purpose of have this comment block repeated. It refers to the names below, but there is nothing below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a bug list of names in _DEPRECATED_IMPORTS below. The difference between these and those that are listed above is that these are deprecated and will give a deprecation warning. The ones above (in this PR currently) are not deprecated and will give no warning when used.



class DeprecatedImportModule:
Copy link
Member

Choose a reason for hiding this comment

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

This should subclass types.ModuleType.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What difference does subclassing ModuleType make?

Copy link
Member

Choose a reason for hiding this comment

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

It makes certain things that operate on modules work correctly. For example, help() gives special output on modules (actually that is wrong anyway, because this doesn't inherit the docstring properly).

I also wonder if this could be an issue

>>> from sympy import ode as ode1
>>> from sympy.solvers import ode as ode2
>>> ode1 == ode2
False

It might or might not be an issue, I'm not sure. I guess if this turns out to cause problems, we can do a patch release that removes it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'd prefer the help to be a message saying that you haven't imported the module in the right way (if there is any need for help at all).

The purpose of providing these deprecated module imports is just to keep code that possibly depends on importing this way chugging along but with warnings. I don't see any need to provide any other functionality.


from sympy.utilities.exceptions import SymPyDeprecationWarning as Warn
import sys
sympy = sys.modules[__name__]

_DEPRECATED_IMPORTS = [
'sympy.concrete.expr_with_intlimits',
'sympy.concrete.expr_with_limits',
'sympy.concrete.gosper',
'sympy.concrete.products',
'sympy.concrete.summations',
'sympy.core.add',
'sympy.core.basic',
'sympy.core.cache',
'sympy.core.compatibility',
'sympy.core.containers',
'sympy.core.coreerrors',
'sympy.core.decorators',
'sympy.core.expr',
'sympy.core.exprtools',
'sympy.core.facts',
'sympy.core.function',
'sympy.core.logic',
'sympy.core.mod',
'sympy.core.mul',
'sympy.core.multidimensional',
'sympy.core.numbers',
'sympy.core.operations',
'sympy.core.power',
'sympy.core.relational',
'sympy.core.rules',
'sympy.core.singleton',
'sympy.core.symbol',
'sympy.discrete.convolutions',
'sympy.geometry.curve',
'sympy.geometry.ellipse',
'sympy.geometry.entity',
'sympy.geometry.exceptions',
'sympy.geometry.line',
'sympy.geometry.parabola',
'sympy.geometry.plane',
'sympy.geometry.point',
'sympy.geometry.polygon',
'sympy.geometry.util',
'sympy.integrals.integrals',
'sympy.integrals.manualintegrate',
'sympy.integrals.meijerint',
'sympy.integrals.singularityfunctions',
'sympy.integrals.transforms',
'sympy.integrals.trigonometry',
'sympy.logic.boolalg',
'sympy.logic.inference',
'sympy.matrices.common',
'sympy.matrices.dense',
'sympy.matrices.expressions',
'sympy.matrices.immutable',
'sympy.matrices.matrices',
'sympy.matrices.sparse',
'sympy.matrices.sparsetools',
'sympy.ntheory.factor_',
'sympy.ntheory.generate',
'sympy.ntheory.multinomial',
'sympy.ntheory.partitions_',
'sympy.ntheory.primetest',
'sympy.ntheory.residue_ntheory',
'sympy.sets.conditionset',
'sympy.sets.contains',
'sympy.sets.fancysets',
'sympy.sets.ordinals',
'sympy.sets.powerset',
'sympy.sets.sets',
'sympy.simplify.cse_main',
'sympy.simplify.cse_opts',
'sympy.simplify.epathtools',
'sympy.simplify.traversaltools',
'sympy.solvers.bivariate',
'sympy.solvers.deutils',
'sympy.solvers.inequalities',
'sympy.solvers.ode',
'sympy.solvers.pde',
'sympy.solvers.polysys',
'sympy.solvers.recurr',
'sympy.solvers.solvers',
'sympy.tensor.array',
'sympy.tensor.index_methods',
'sympy.tensor.indexed'
]

def __init__(self, modname):
from importlib import import_module
self.modname = modname
self.mod = import_module(modname)

def __getattr__(self, name):
self.Warn(
feature="importing %s with 'from sympy import *'" % self.modname,
useinstead="import %s" % self.modname,
issue=18245,
deprecated_since_version="1.6").warn()
return getattr(self.mod, name)

@classmethod
def inject_imports(cls):
for modname in cls._DEPRECATED_IMPORTS:
name = modname.split('.')[-1]
deprecated_mod = cls(modname)
setattr(cls.sympy, name, deprecated_mod)
__all__.append(name)


DeprecatedImportModule.inject_imports()
del DeprecatedImportModule
26 changes: 26 additions & 0 deletions sympy/deprecated/tests/test_deprecated_imports.py
@@ -0,0 +1,26 @@
import sympy
from sympy.testing.pytest import warns_deprecated_sympy

def test_deprecated_imports():
# https://github.com/sympy/sympy/pull/18245
# Before 1.6 these names were importable with e.g.
# from sympy import *
# from sympy import add
# Now sympy/__init__.py uses __all__ so these names are no longer
# accidentally imported. However many of the names now give a warning and
# this test checks that they are importable but a warning is given
from sympy import add

with warns_deprecated_sympy():
add.Add

modnames = type(add)._DEPRECATED_IMPORTS

assert len(modnames) == 80

for modname in modnames:
name = modname.split('.')[-1]
mod = getattr(sympy, name)
attr = dir(mod.mod)[0]
with warns_deprecated_sympy():
getattr(mod, attr)