From 575be23d4994cabb0ad7b9469a319c1758b10a15 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Sat, 27 Nov 2021 10:11:01 -0800 Subject: [PATCH 1/3] Don't add ipython unconditionally to sys.path Load ext will already do it with a context manager. This should remove false positive when import modules, whithout breaking functionalities. Maybe closes #13294 (making this part of 8.0 release make sens). --- IPython/core/application.py | 40 +++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/IPython/core/application.py b/IPython/core/application.py index 85c91274d4e..3a1797ab090 100644 --- a/IPython/core/application.py +++ b/IPython/core/application.py @@ -185,6 +185,15 @@ def _profile_changed(self, change): get_ipython_package_dir(), u'config', u'profile', change['new'] ) + add_ipython_dir_to_sys_path = Bool(False, + """Should the IPython profile directory be added to sys path ? + + This option was non-existing before IPython 8.0, and ipython_dir was added to + sys path to allow import of extensions present there. This was historical + baggage from when pip did not exist. This now default to false, + but can bet to true for legacy reasons + """).tag(config=True) + ipython_dir = Unicode( help=""" The name of the IPython directory. This directory is used for logging @@ -294,21 +303,22 @@ def _ipython_dir_changed(self, change): str_old = os.path.abspath(old) if str_old in sys.path: sys.path.remove(str_old) - str_path = os.path.abspath(new) - sys.path.append(str_path) - ensure_dir_exists(new) - readme = os.path.join(new, 'README') - readme_src = os.path.join(get_ipython_package_dir(), u'config', u'profile', 'README') - if not os.path.exists(readme) and os.path.exists(readme_src): - shutil.copy(readme_src, readme) - for d in ('extensions', 'nbextensions'): - path = os.path.join(new, d) - try: - ensure_dir_exists(path) - except OSError as e: - # this will not be EEXIST - self.log.error("couldn't create path %s: %s", path, e) - self.log.debug("IPYTHONDIR set to: %s" % new) + if self.add_ipython_dir_to_sys_path: + str_path = os.path.abspath(new) + #sys.path.append(str_path) + ensure_dir_exists(new) + readme = os.path.join(new, 'README') + readme_src = os.path.join(get_ipython_package_dir(), u'config', u'profile', 'README') + if not os.path.exists(readme) and os.path.exists(readme_src): + shutil.copy(readme_src, readme) + for d in ('extensions', 'nbextensions'): + path = os.path.join(new, d) + try: + ensure_dir_exists(path) + except OSError as e: + # this will not be EEXIST + self.log.error("couldn't create path %s: %s", path, e) + self.log.debug("IPYTHONDIR set to: %s" % new) def load_config_file(self, suppress_errors=IPYTHON_SUPPRESS_CONFIG_ERRORS): """Load the config file. From 73ffd1ba1d3e83b0611df6ec0f5ec3eb12c5bf30 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Wed, 1 Dec 2021 11:51:18 -0800 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Min RK --- IPython/core/application.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/IPython/core/application.py b/IPython/core/application.py index 3a1797ab090..337f7ba7f7c 100644 --- a/IPython/core/application.py +++ b/IPython/core/application.py @@ -191,7 +191,7 @@ def _profile_changed(self, change): This option was non-existing before IPython 8.0, and ipython_dir was added to sys path to allow import of extensions present there. This was historical baggage from when pip did not exist. This now default to false, - but can bet to true for legacy reasons + but can be set to true for legacy reasons. """).tag(config=True) ipython_dir = Unicode( @@ -305,7 +305,7 @@ def _ipython_dir_changed(self, change): sys.path.remove(str_old) if self.add_ipython_dir_to_sys_path: str_path = os.path.abspath(new) - #sys.path.append(str_path) + sys.path.append(str_path) ensure_dir_exists(new) readme = os.path.join(new, 'README') readme_src = os.path.join(get_ipython_package_dir(), u'config', u'profile', 'README') From 96c5d75c72131f888b8d3f9dc95a7fc35ee68808 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Wed, 1 Dec 2021 11:52:17 -0800 Subject: [PATCH 3/3] reformat --- IPython/core/application.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/IPython/core/application.py b/IPython/core/application.py index 337f7ba7f7c..c9e8e6f4d43 100644 --- a/IPython/core/application.py +++ b/IPython/core/application.py @@ -185,14 +185,16 @@ def _profile_changed(self, change): get_ipython_package_dir(), u'config', u'profile', change['new'] ) - add_ipython_dir_to_sys_path = Bool(False, + add_ipython_dir_to_sys_path = Bool( + False, """Should the IPython profile directory be added to sys path ? This option was non-existing before IPython 8.0, and ipython_dir was added to sys path to allow import of extensions present there. This was historical baggage from when pip did not exist. This now default to false, but can be set to true for legacy reasons. - """).tag(config=True) + """, + ).tag(config=True) ipython_dir = Unicode( help=""" @@ -307,11 +309,13 @@ def _ipython_dir_changed(self, change): str_path = os.path.abspath(new) sys.path.append(str_path) ensure_dir_exists(new) - readme = os.path.join(new, 'README') - readme_src = os.path.join(get_ipython_package_dir(), u'config', u'profile', 'README') + readme = os.path.join(new, "README") + readme_src = os.path.join( + get_ipython_package_dir(), "config", "profile", "README" + ) if not os.path.exists(readme) and os.path.exists(readme_src): shutil.copy(readme_src, readme) - for d in ('extensions', 'nbextensions'): + for d in ("extensions", "nbextensions"): path = os.path.join(new, d) try: ensure_dir_exists(path)