#3 Implement IPython REPL #7

anxolerd · 2017-05-09T16:09:28Z

Added blueberrypy ishell command

Implement #3

anxolerd · 2017-05-09T20:26:25Z

It seems IPython 6.0.0 is not available for Python2.7 Do you plan to support Python2 ?
If no, should I downgrade IPython to lower version?

webknjaz · 2017-05-10T07:08:52Z

src/blueberrypy/command.py

@@ -445,6 +483,8 @@ def get_command_parser_and_callback(command):
            doc, callback = create.__doc__, create
        elif command == "console":
            doc, callback = console.__doc__, console
+        elif command == "ishell":


I'd go for extending existing command with autonegotiation of REPL, available for use. See django-extensions - > shell_plus

webknjaz · 2017-05-10T07:09:44Z

setup.py

@@ -10,7 +10,20 @@
                    "docopt>=0.6",
                    "Jinja2>=2.7",
                    "PyYAML>=3.10",
-                    "python-dateutil>=2.2"]
+                    "python-dateutil>=2.2",
+                    # IPython dependencies:


Why would you need to lock recursive deps?

Because I do not need any surprises of unexpected updates.
Do you think I should not do that and lock only IPython dependency?

In a project of a library type you shouldn't restrict versioning strictly unless for corner cases.

Well, I do not trust them enough to do that.

But if you wish I may fix that if you tell me which versions should I allow.

webknjaz · 2017-05-10T07:11:02Z

should I downgrade

There's environment markers for this purpose

webknjaz

plz also move ipython to extras

webknjaz · 2017-05-11T07:48:11Z

setup.py

-                    "traitlets==4.3.2",
-                    "wcwidth==0.1.7"]
+                    "ipython==5.3.0" if sys.version_info < (3, 3)
+                    else "ipython==6.0.0"]


webknjaz · 2017-05-11T07:48:18Z

setup.py

-                    "simplegeneric==0.8.1",
-                    "traitlets==4.3.2",
-                    "wcwidth==0.1.7"]
+                    "ipython==5.3.0" if sys.version_info < (3, 3)


webknjaz · 2017-05-11T07:50:42Z

src/blueberrypy/command.py


 import cherrypy
+from IPython.terminal.interactiveshell import TerminalInteractiveShell


this will fail if ipython isn't installed

Ipython is in core dependencies so I guess it will be installed with the blueberrypy

and it shouldn't be there

Why shouldn't it?

because it's not a runtime dependency, it should be optional; if one wants to use ipython they'll install it.

webknjaz · 2017-05-11T07:53:38Z

can you borrow the whole logic from django-extensions?

anxolerd · 2017-05-11T07:56:54Z

can you borrow the whole logic from django-extensions?

What do you mean by that?

webknjaz · 2017-05-11T07:58:04Z

copy-paste driven development, I think

anxolerd · 2017-05-11T08:03:49Z

copy-paste driven development, I think

I do not understand what exactly you want me to copy-paste. Other shells support? Or what?

anxolerd · 2017-06-20T21:15:47Z

Updated PR. @webknjaz pls review again =)

webknjaz

So what about other shells?

webknjaz · 2017-06-22T04:52:03Z

setup.py

@@ -75,7 +75,10 @@
                              "Routes>=2.0",
                              "backlash>=0.0.5",
                              "Shapely>=1.2",
-                              "GeoAlchemy2>=0.2.4"],
+                              "GeoAlchemy2>=0.2.4",
+                              "ipython<=5.3.0" if sys.version_info < (3, 3) else "ipython>=6.0.0"],


This conditional must be computed once and passed via variable; alternatively, you may wrap it into lambda. Copy-pasted code snippets lead to unexpected diversity increase of code base over time.

webknjaz · 2017-06-22T04:54:16Z

src/blueberrypy/command.py


 import cherrypy
 from docopt import docopt
 from cherrypy.process import servers
 from cherrypy.process.plugins import Daemonizer, DropPrivileges, PIDFile

 import blueberrypy
+import blueberrypy.shell as shell


Isn't from blueberrypy import shell shorter?

Yes, it is.

webknjaz · 2017-06-22T04:57:39Z

src/blueberrypy/shell.py

+
+
+def get_package_name(config):
+    return (


getattr(config, 'project_metadata', {}).get('package', None)

I think my version is more explicit.

webknjaz · 2017-06-22T04:58:47Z

src/blueberrypy/shell.py

+    )
+
+
+def _make_sa_engine(config):


Can't this code be shared with SQLAlchemy Tool module?

No it cannot be shared properly =(

anxolerd · 2017-06-25T14:33:05Z

I think we'll stop on IPython for now. Will add other shells on demand.

anxolerd · 2017-06-25T14:35:02Z

@webknjaz:

removed code duplication from setup.py
shorter import is used

I do not see any good way to share code with SQLAlchemy tool without overcomplicating the tool itself.

webknjaz · 2017-06-25T16:46:27Z

src/blueberrypy/shell.py

+    ns['cherrypy'] = importlib.import_module("cherrypy")
+    ns['blueberrypy'] = importlib.import_module("blueberrypy")
+
+    if config.use_sqlalchemy:


I think you could reuse this property https://github.com/open-craft-guild/blueberrypy/blob/master/src/blueberrypy/config.py#L270

The logic I used was present in old blueberrypy console. This is why I kept it.

webknjaz · 2017-06-25T16:58:05Z

src/blueberrypy/shell.py

+
+
+def get_user_namespace(config, include_pkg=False):
+    assert isinstance(config, BlueberryPyConfiguration), type(config)


type(config) doesn't look like sufficient error message

Agree. Fixed in 44e80e3

anxolerd · 2017-06-26T14:30:48Z

@webknjaz can this be merged, or any further changes are required?

webknjaz · 2017-06-26T14:31:40Z

Go ahead and merge it.

…

On Jun 26, 2017 17:30, "Oleksandr Kovalchuk" ***@***.***> wrote: @webknjaz <https://github.com/webknjaz> can this be merged, or any further changes are required? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAjT71JfY9JgNkqslyOIbtp93ysjmMU4ks5sH8CYgaJpZM4NVipO> .

anxolerd · 2017-06-26T14:39:19Z

@webknjaz I am not authorized to merge this pull request.

webknjaz · 2017-06-26T19:56:17Z

This branch is out of sync with the base branch.
Changes from the base branch must be synced with this branch before merging.

Fix this first

webknjaz requested changes May 10, 2017

View reviewed changes

webknjaz requested changes May 11, 2017

View reviewed changes

anxolerd force-pushed the 3-ipython-repl branch from 09f361a to d7ff46b Compare June 20, 2017 21:13

webknjaz requested changes Jun 22, 2017

View reviewed changes

webknjaz reviewed Jun 25, 2017

View reviewed changes

webknjaz approved these changes Jun 26, 2017

View reviewed changes

anxolerd force-pushed the 3-ipython-repl branch from 44e80e3 to 6df068c Compare June 26, 2017 20:57

webknjaz closed this Jun 28, 2017

webknjaz force-pushed the 3-ipython-repl branch from 6df068c to b0bbed0 Compare June 28, 2017 12:20


		import cherrypy
		from IPython.terminal.interactiveshell import TerminalInteractiveShell



		def get_user_namespace(config, include_pkg=False):
		assert isinstance(config, BlueberryPyConfiguration), type(config)

#3 Implement IPython REPL #7

#3 Implement IPython REPL #7

Conversation

anxolerd commented May 9, 2017 • edited Loading

anxolerd commented May 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webknjaz commented May 10, 2017

webknjaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webknjaz commented May 11, 2017

anxolerd commented May 11, 2017

webknjaz commented May 11, 2017

anxolerd commented May 11, 2017 • edited Loading

anxolerd commented Jun 20, 2017

webknjaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anxolerd commented Jun 25, 2017

anxolerd commented Jun 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anxolerd commented Jun 26, 2017

webknjaz commented Jun 26, 2017 via email

anxolerd commented Jun 26, 2017

webknjaz commented Jun 26, 2017

anxolerd commented May 9, 2017 •

edited

Loading

anxolerd commented May 11, 2017 •

edited

Loading