Skip to content

Modification of attribute banner to fix issue with launching with qtconsole command #14

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hansonap
Copy link

See issue #13 for original bug details.

Updated the PowerShellKernel to use the traitlets model for the attribute banner. I've never used traitlets before, but that what the underlining ipykernel.Kernel is structured off of and what qtconsole.base_frontend_mixin.BaseFrontendMixin's _dispatch() is expecting.

Implementation:

  1. banner's type is set with banner = Unicode()
  2. banner's default value is provided with:
    @default('banner')
    def _banner_default(self):
        return check_output(['powershell', '$PSVersionTable.PSVersion']).decode('utf-8')
    
  3. no other traitlets methods were implemented

Questions for @vors

  1. Do you want me to increase the version in kernel.py? If so I assume 0.0.8, is that correct?
  2. Since the traitlets package is now being used, do I need to update any packaging requirements (I've contributed to code installable with pip before - a little out of my depth)?
  3. The following packages are marked as unused; do you want me to remove them?
    • os.unlink
    • base64
    • imghdr
    • signal
    • urllib

Other comments:

  1. As a result of fixing this the command line argument --FrontendWidget.banner=... now works, however this is the JupyterWidget(IPythonWidget)'s banner, not the kernel's banner.

issue vors#13 for more details.

Updated the PowerShellKernel to use traitlets for attribute banner.
@vors
Copy link
Owner

vors commented Jan 14, 2019

  1. Yes, let's bump the version
  2. That's a very good question, I haven't publish anything to pip that has a dependencies. I assume we need a requirements.txt now to support it. I'd very much appreciate if you figure out this story and share with me.
  3. Sure, feel free to remove them. Please use a separate PR for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants