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

Add Python package sos-notebook. #7665

Merged
merged 18 commits into from
Apr 23, 2019
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions recipes/sos-notebook/ex.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x=1

65 changes: 65 additions & 0 deletions recipes/sos-notebook/meta.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
{% set name = "sos-notebook" %}
{% set version = "0.19.1" %}

package:
name: "{{ name|lower }}"
version: "{{ version }}"

source:
url: https://pypi.io/packages/source/{{ name[0] }}/{{ name }}/{{ name }}-{{ version }}.tar.gz
sha256: 9f3ae4371f2a9cf493c77190010e73270ddc658904618937889ce14c0ecebb09

build:
Copy link
Member

Choose a reason for hiding this comment

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

Please, delete the build.sh and bld.bat. After that please add in your build section

  script:
    - {{ PYTHON }} -m pip install . --no-deps --ignore-installed -vv
    - {{ PYTHON }} -m sos_notebook.install --sys-prefix

Otherwise, this recipe cannot be noarch

Copy link
Member Author

Choose a reason for hiding this comment

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

@marcelotrevisani Thanks! I just pushed this change.

noarch: python
number: 0
script:
- {{ PYTHON }} -m pip install . --no-deps --ignore-installed -vv
- {{ PYTHON }} -m sos_notebook.install --sys-prefix

requirements:
host:
- ipython
- notebook >=5.0.0
- pip
- python >=3.6
Copy link
Member

Choose a reason for hiding this comment

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

If this can't be noarch you should add skip: True # [py<36] to the build section and specify remove the version constraint from Python:

Suggested change
- python >=3.6
- python

(same applies in the run section)

Copy link
Member

Choose a reason for hiding this comment

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

Having looked more closely it looks like this could be noarch: python?

run:
- ipykernel
- ipython
- markdown
- matplotlib
- nbconvert >=5.1.1
- nbformat
- notebook >=5.0.0
- pandas
- python >=3.6
- setuptools
Copy link
Contributor

Choose a reason for hiding this comment

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

setuptools is not needed for run.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then should I move it to host? It is called in setup.py, so I assume it is needed for installation.

https://github.com/vatlab/sos-notebook/blob/master/setup.py#L11

- sos >=0.19.2
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, fiddling this for a long time... because sos is not in the default channel, and conda build does not yet support specifying channels for requirements, I will have to run

conda config --add channels conda-forge 

before running

conda build sos-notebook

It is a shame that conda-forge does not allow dependency from conda-forge.

Copy link
Member

Choose a reason for hiding this comment

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

It is a shame that conda-forge does not allow dependency from conda-forge.

Huh? This is not true

Copy link
Contributor

Choose a reason for hiding this comment

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

As I described, conda-forge packages are allowed, but users have to run

conda config --add channels conda-forge

outside of conda build, rather than allowing things like

channels:
   - conda-forge

or

requirements:
    - conda-forge:
        - sos

in the configuration file.

I apologize for my "shame" word but it was frustrating to spend a few hours debugging this simple problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

At least the error message could be more informative such as "package xxx cannot be found in channels xxxx".

- tabulate

test:
requires:
- nose
imports:
- sos_notebook
commands:
- jupyter kernelspec list | grep -w $PREFIX/share/jupyter/kernels/sos # [unix]
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not work under mac here. It seems that | split the commands into two.

+ jupyter kernelspec list
+ grep -w /Users/bpeng1/anaconda3/conda-bld/sos-notebook_1554269979093/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_plac/share/jupyter/kernels/sos
Tests failed for sos-notebook-0.19.1-py_0.tar.bz2 - moving package to /Users/bpeng1/anaconda3/conda-bld/broken
WARNING:conda_build.build:Tests failed for sos-notebook-0.19.1-py_0.tar.bz2 - moving package to /Users/bpeng1/anaconda3/conda-bld/broken
removing: sos-notebook-0.19.1-py_0.tar.bz2
TESTS FAILED: sos-notebook-0.19.1-py_0.tar.bz2

Copy link
Contributor

Choose a reason for hiding this comment

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

@isuruf I would also appreciate your guidance here. Did | really separate the command into two? Then the second grep command should hang indefinitely.

Copy link
Member

Choose a reason for hiding this comment

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

No, it doesn't split the command into two. It's just that when the commands are echoed they are shown like that

Copy link
Contributor

Choose a reason for hiding this comment

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

Figured out the problem, the build and test steps use different environments with different python.

@jdblischak Please change

    - jupyter kernelspec list | grep -w $PREFIX/share/jupyter/kernels/sos  # [unix]
    - jupyter kernelspec list | findstr \share\jupyter\kernels\sos  # [win]

to

    - jupyter kernelspec list | grep kernels/sos  # [unix]
    - jupyter kernelspec list | findstr kernels\sos  # [win]

Copy link
Member Author

Choose a reason for hiding this comment

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

@BoPeng I don't understand your concern. This test passes on macOS, which you can see at the line below:

https://travis-ci.org/conda-forge/staged-recipes/builds/513186637#L1626

Copy link
Contributor

Choose a reason for hiding this comment

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

It failed locally on my mac. I think the problem was that I have locally installed kernels and multiple jupyter commands so jupyter kernelspec list returned my local specs. The test machine does not have this problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm....that still doesn't seem right. Even though you have local kernels installed, the sos kernel should still be installed in the conda environment, and thus the grep command should find it. I'm not sure what is going on. Is the command python -m sos_notebook.install --sys-prefix affected by the presence of locally installed kernels? (e.g. maybe it won't install if it already exists locally?)

Copy link
Contributor

Choose a reason for hiding this comment

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

The jupyter command used in the test might be from local environment. The change can be useful because it makes a bit easier to test the recipe locally, even if the wrong jupyter or python was used. Not a big deal though.

Copy link
Contributor

Choose a reason for hiding this comment

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

The jupyter command used in the test might be from local environment. The change can be useful because it makes a bit easier to test the recipe locally, even if the wrong jupyter or python was used. Not a big deal though.

Copy link
Member Author

Choose a reason for hiding this comment

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

The jupyter command used in the test might be from local environment.

That shouldn't happen. In a conda environment, the first jupyter found via PATH should always be the one installed as part of the conda environment.

The change can be useful because it makes a bit easier to test the recipe locally, even if the wrong jupyter or python was used.

If the wrong jupyter or python is being used, it is good that the test fails since this indicates something is wrong with the isolation of the conda environment.

- jupyter kernelspec list | findstr \share\jupyter\kernels\sos # [win]
- jupyter run --kernel=sos ex.py
files:
- ex.py

about:
home: https://github.com/vatlab/SOS
license: BSD
license_family: BSD
license_file: LICENSE
summary: "Script of Scripts (SoS): an interactive, cross-platform, and
cross-language workflow system for reproducible data analysis"
doc_url: https://vatlab.github.io/sos-docs/
dev_url: https://github.com/vatlab/sos-notebook

extra:
recipe-maintainers:
- BoPeng
- gaow
- jdblischak
29 changes: 29 additions & 0 deletions recipes/sos-notebook/run_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Test that sos kernel is installed

import jupyter_client

try:
jupyter_client.kernelspec.get_kernel_spec('sos')
except jupyter_client.kernelspec.NoSuchKernel:
print('sos kernel was not installed')
print('The following kernels are installed:')
print('jupyter_client.kernelspec.find_kernel_specs()')
print(jupyter_client.kernelspec.find_kernel_specs())

# Test that sos kernel is functional

import unittest

from ipykernel.tests.utils import execute, wait_for_idle, assemble_output
from sos_notebook.test_utils import sos_kernel

class TestSoSKernel(unittest.TestCase):
def testKernel(self):
with sos_kernel() as kc:
execute(kc=kc, code='a = 1\nprint(a)')
stdout, stderr = assemble_output(kc.iopub_channel)
self.assertEqual(stderr, '')
self.assertEqual(stdout.strip(), '1')

if __name__ == '__main__':
unittest.main()