Skip to content

Conversation

@healther
Copy link
Contributor

  • add setuptools dependency for newer version
    the whole jupyter collection seems to use setuptools in case of
    certain setup.py-arguments from the very beginning. However the latest
    ones actually require it, otherwise the build will fail
  • add newly introduced dependencies

@adamjstewart
Copy link
Member

For the record, I'm renaming py-jupyter-notebook to py-notebook in #13411

@healther
Copy link
Contributor Author

Same question as in the other PR: Which merge-order do you prefer?

@adamjstewart
Copy link
Member

No preference, we just need to remember to update based on which is merged first.


variant('terminal', default=False, description="Enable terminal functionality")

depends_on('python@2.7:2.8,3.3:')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
depends_on('python@2.7:2.8,3.3:')
depends_on('python@2.7:2.8,3.3:', type=('build', 'run'))
depends_on('python@3.5:', when='@6:', type=('build', 'run'))

It's probably worth adding the latest version of Notebook 5.X so we have a more up-to-date version for Python 2 users.

depends_on('python@2.7:2.8,3.3:')
depends_on('py-setuptools', type='build', when='@6:')
depends_on('py-jinja2', type=('build', 'run'))
depends_on('py-tornado@4:', type=('build', 'run'))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
depends_on('py-tornado@4:', type=('build', 'run'))
depends_on('py-tornado@5.0:', type=('build', 'run'))

# right now treat them as 6-or-newer dependencies
depends_on('py-prometheus-client', type=('build', 'run'), when='@6:')
depends_on('py-terminado', type=('build', 'run'), when='@6:')
depends_on('py-send2trash', type=('build', 'run'), when='@6:')
Copy link
Member

Choose a reason for hiding this comment

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

Missing dependencies and version constraints on:

        'pyzmq>=17',                                                            
        'traitlets>=4.2.1',                                                     
        'jupyter_core>=4.4.0',                                                  
        'jupyter_client>=5.3.1',    
        'terminado>=0.8.1',                                                     
        ':python_version == "2.7"': ['ipaddress'],                              

Also, py-jupyter-console is not required.

@adamjstewart
Copy link
Member

Ping @healther, needs a rebase

* add setuptools dependency for newer version
  the whole jupyter collection seems to use setuptools in case of
  certain setup.py-arguments from the very beginning. However the latest
  ones actually require it, otherwise the build will fail
* add newly introduced dependencies
@healther
Copy link
Contributor Author

healther commented Nov 4, 2019

Done, requires the other py-jupyter stuff to be merged first

@healther
Copy link
Contributor Author

healther commented Nov 4, 2019

Travis fail is unrelated, but a libiconv checksum error seems weird

@adamjstewart adamjstewart changed the title update py-jupyter-notebook update py-notebook Nov 4, 2019

variant('terminal', default=False, description="Enable terminal functionality")

depends_on('python@2.7:2.8,3.3:')
Copy link
Member

Choose a reason for hiding this comment

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

The latest version only supports Python 3.5+. Also, need type=('build', 'run').

version('4.0.4', sha256='a57852514bce1b1cf41fa0311f6cf894960cf68b083b55e6c408316b598d5648')
version('4.0.2', sha256='8478d7e2ab474855b0ff841f693983388af8662d3af1adcb861acb900274f22a')

variant('terminal', default=False, description="Enable terminal functionality")
Copy link
Member

Choose a reason for hiding this comment

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

I would honestly just get rid of this variant. It isn't present in the setup.py, and terminado was never required until recent versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure about this as it might break older things (and presumably people cared enough to add it), it was there till the very beginning.

Anyways: Semi-related side rant: github history does not work over renames so git log var/spack/repos/builtin/packages/py-notebook/package.py only works till 2264e30

@adamjstewart adamjstewart merged commit 74c81c0 into spack:develop Nov 5, 2019
@healther healther deleted the PR/pynotebook branch November 5, 2019 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants