Cesc/update recipe #2

FrancescElies · 2016-11-02T22:28:54Z

bumps version, adds some fixes, a maintainer and a todo for the test section

conda-forge-linter · 2016-11-02T22:28:57Z

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

The recipe must have some tests.

conda-forge-linter · 2016-11-02T22:55:11Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

mrocklin · 2016-11-02T23:39:07Z

Just curious, is Windows out of the question for blosc?

jakirkham · 2016-11-03T02:00:49Z

That's a good question for @msarahan .

jakirkham · 2016-11-03T05:24:40Z

The failure on Travis CI is due to the feedstock needing an update. Please re-render with conda-smithy version 1.4.6 to address this. Can add -c/--commit to conda smithy rerender to commit the re-rendering.

FrancescElies · 2016-11-03T07:13:31Z

Thanks for the help, rerendering added missing appveyor file for windows

jakirkham · 2016-11-03T13:00:39Z

recipe/meta.yaml

@@ -1,39 +1,44 @@
-{% set version = "1.3.2" %}
+{% set name = "blosc" %}


Why the name change? Shouldn't this still be python-blosc? Note we do package blosc also.

When you install one wants to run it you run, import blosc I thought this would be less confusing, but the repo is called python-blosc. Changing the name back to python-blosc
e0f0754

jakirkham · 2016-11-03T13:01:04Z

recipe/meta.yaml

  run:
    - python
-    - blosc


Why are we dropping blosc as a dependency?

I was doubting on this, I could not understand why do we need the package itself as a dependency?
1725c1a

blosc is refering to the c-blosc lib, the recipe for that one is here: https://github.com/conda-forge/blosc-feedstock. This one python-blosc is just the python wrapper. We decided on those names to have the same ones as the defaults channel, here is the full discussion: conda-forge/staged-recipes#918

So python-blosc will build c-blosc if its not found (in the setup.py) but if we add it as a dependency it will just link it to the c-blosc that we build.

I guess the question will arise how we build c-blosc/blosc on Windows.

@msarahan can you please advise us on this? 😄

jakirkham · 2016-11-03T13:01:29Z

recipe/meta.yaml

    - numpy

 test:
-  imports:
+  requires:
    - blosc


jakirkham · 2016-11-03T13:01:47Z

recipe/meta.yaml

    - blosc
  commands:
-    - python -c "import blosc; blosc.test()"
+    - mkdir empty
+    - cd empty


Why is this needed?

In my opinion we don't need it, I was just following the travis.ci file from the python-blosc project. so removing it
390e29d

jakirkham · 2016-11-03T13:01:54Z

recipe/meta.yaml


 about:
  home: https://github.com/Blosc/python-blosc
  license: Apache 2.0
+  license_file: LICENSES/PYTHON-BLOSC.txt


jakirkham · 2016-11-03T13:02:22Z

recipe/meta.yaml

+    - mkdir empty
+    - cd empty
+    - python -c 'import blosc; blosc.print_versions()'
+    - python -c 'import blosc; blosc.test()'

 about:
  home: https://github.com/Blosc/python-blosc
  license: Apache 2.0


We could also optionally add license_family: Apache.

I believe I still need some input for the license, apparently there are some plans to change it to bsd Blosc/python-blosc#121 (comment)

Adding it acbca92

jakirkham · 2016-11-03T13:03:10Z

Ah, yeah, if you change skip, then re-rendering is needed to update the CI files included.

danielfrg · 2016-11-03T16:35:07Z

README.md

@@ -1,5 +1,5 @@
-About python-blosc


I guess that is is because of the rerendering of the feedstock? Maybe you need to rerender again.

Correct. After fixing the name we will need another re-render. Note can do conda smithy rerender --commit to commit the changes. Also can tack on auto to commit the re-rendering automatically without reviewing the changes.

danielfrg · 2016-11-03T16:35:38Z

+1 Great to see a maintainer come in for this one :D

jakirkham · 2016-11-03T17:09:18Z

Could you please disable CircleCI on your fork @FrancescElies ? It seems to be building this PR on your fork instead of on this feedstock.

jakirkham · 2016-11-03T17:11:05Z

Also please close and reopen this PR after you have done that. This will trigger CircleCI to correctly build on this PR.

FrancescElies · 2016-11-03T17:23:42Z

I deactivated Circle Ci, Travis and Appveyor from my profile

FrancescElies · 2016-11-03T17:24:02Z

By the way, thank you both for your help!

FrancescElies · 2016-11-03T17:25:30Z

Reopened to trigger CI services

FrancescElies · 2016-11-03T17:30:10Z

recipe/meta.yaml


 requirements:
  build:
    - python
    - setuptools
-    - blosc


Shall I add this build requirements again?
The git history is a bit messy, once this PR is fine I could clean the history if you like.

Yes, let's go ahead and do that.

Sure if you want to clean it up at the end that sounds like a good idea.

msarahan · 2016-11-03T17:37:10Z

We have a c-blosc recipe internally, but I don't see packages, and I don't understand why. Internally, I think we decided to change the compiled library to c-blosc, because that matches the project name.

FrancescElies · 2016-11-03T17:41:42Z

Now I just remembered why I changed the name of the package, from python-blosc to blosc
If I try to download https://pypi.io/packages/source/p/python-blosc/python-blosc-1.4.4.tar.gz it cannot be found
But if I try to this onehttps://pypi.io/packages/source/b/blosc/blosc-1.4.4.tar.gz seems to be fine.

jakirkham

Naming is causing us some issues with the download. As it is called blosc on PyPI, but we call it python-blosc here, we need to make a few corrections.

jakirkham · 2016-11-03T17:44:07Z

recipe/meta.yaml


 package:
-  name: python-blosc
+  name: {{ name|lower }}


Maybe we should make this python-{{ name|lower }}.

sounds good to me

jakirkham · 2016-11-03T17:44:21Z

recipe/meta.yaml

@@ -1,39 +1,44 @@
-{% set version = "1.3.2" %}
+{% set name = "python-blosc" %}


Maybe we should change this to blosc.

jakirkham · 2016-11-03T17:44:31Z

recipe/meta.yaml

-  url: https://github.com/Blosc/python-blosc/archive/v{{ version }}.tar.gz
-  sha256: 63d1cd1da14087fa69bffc513b25d59deb24f45c65e4406acafaaa6ec9bd7cf2
+  fn: {{ name }}-{{ version }}.tar.gz
+  url: https://pypi.io/packages/source/{{ name[0] }}/{{ name }}/{{ name }}-{{ version }}.tar.gz


This then should work.

jakirkham · 2016-11-03T18:17:41Z

Seems our comments passed each other. My comments should address the download issue.

jakirkham · 2016-11-03T20:27:00Z

So I think this is mostly good now. The one issue is that we still don't have a blosc package for Windows. We have a few options.

Try to build a blosc package on Windows and come back to this.
Go back to skipping Windows.
Supply blosc on Unix and let Windows build blosc with the Python package.

Not sure which will be best.

FrancescElies · 2016-11-03T20:34:53Z

I'm not sure, to me the cleanest sounds (1.), but maybe for the moment we could skip windows. I'm easy

FrancescElies · 2016-11-04T11:24:28Z

I actually have some questions
Before we were using python setup.py install --blosc=$PREFIX --single-version-externally-managed --record record.txt to install python-blosc but I could not see in the project where the PREFIX for blosc is being set.
The current status of the recipe does not use PREFIX therefore when we install python-blosc it will use the blosc sources that are being shipped with python-blosc
Then, do we sill need blosc as dependency?
In python-blosc the recommended way is compiling with an installed Blosc library setting BLOSC_DIR env var, what shall we do?

danielfrg · 2016-11-04T15:55:24Z

PREFIX is set by conda-build. If the recommended way is to install blosc, link and then use BLOSC_DIR instead of --blosc=... we should change it to use that. What should the value of BLOSC_DIR the path to the compiled binaries?

FrancescElies · 2016-11-04T16:05:02Z

--blosc=$PREFIX has the same effect as setting BLOSC_DIR.
It should be set to the path where blosc is being installed, from what I can see https://github.com/conda-forge/blosc-feedstock/blob/master/recipe/build.sh#L6 is being set to PREFIX, it seems like it should work.
I'll give it a try

jakirkham · 2016-11-04T16:11:20Z

I'm not sure, to me the cleanest sounds (1.), but maybe for the moment we could skip windows. I'm easy

Me too. 😄 Would love to get it to work on Windows at some point though. 😉

FrancescElies · 2016-11-04T16:31:25Z

It looks like something wen't wrong now,
Error: HTTPError: 503 Server Error: Service Unavailable: Back-end server is at capacity for url: https://conda.anaconda.org/conda-forge/linux-64/openblas-0.2.18-6.tar.bz2: https://conda.anaconda.org/conda-forge/linux-64/openblas-0.2.18-6.tar.bz2
I'll give a try to blosc on windows next week

FrancescElies · 2016-11-04T16:31:48Z

regtriggering CI services

mrocklin · 2016-11-30T23:18:30Z

Recommend retriggering CI services again

conda-forge-linter · 2016-12-01T08:20:46Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

FrancescElies · 2016-12-01T08:22:00Z

Retriggered (close-open PR). I see appveyor and travis running again but not circle ci

FrancescElies · 2016-12-01T08:31:12Z

an empty commit retriggered circle ci (I'll remove it afterwards)

FrancescElies · 2017-01-31T10:08:31Z

removed empty commit, + rerendering, fingers crossed

FrancescElies · 2017-02-07T19:30:28Z

tests passed now

FrancescElies mentioned this pull request Nov 2, 2016

Maintain conda packages on conda forge Blosc/python-blosc#118

Closed

jakirkham reviewed Nov 3, 2016

View reviewed changes

danielfrg reviewed Nov 3, 2016

View reviewed changes

FrancescElies closed this Nov 3, 2016

FrancescElies reopened this Nov 3, 2016

FrancescElies commented Nov 3, 2016

View reviewed changes

jakirkham reviewed Nov 3, 2016

View reviewed changes

FrancescElies force-pushed the cesc/update-recipe branch 2 times, most recently from e2c0ca4 to 59b4fbb Compare November 3, 2016 20:22

FrancescElies force-pushed the cesc/update-recipe branch from 59b4fbb to 6bdfb32 Compare November 3, 2016 20:23

FrancescElies force-pushed the cesc/update-recipe branch from 6bdfb32 to 98e148d Compare November 3, 2016 20:27

FrancescElies force-pushed the cesc/update-recipe branch from f83563f to 7115989 Compare November 4, 2016 16:23

FrancescElies closed this Nov 4, 2016

FrancescElies reopened this Nov 4, 2016

FrancescElies closed this Dec 1, 2016

FrancescElies reopened this Dec 1, 2016

FrancescElies force-pushed the cesc/update-recipe branch from bed8d7d to 403f841 Compare January 31, 2017 10:07

FrancescElies added 2 commits February 5, 2017 22:47

updates recipe + bumps version

09eb746

MNT: Re-rendered with conda-smithy 2.0.1

d0ab9d9

FrancescElies force-pushed the cesc/update-recipe branch from 403f841 to d0ab9d9 Compare February 5, 2017 21:47

danielfrg merged commit bf2e96d into conda-forge:master Feb 25, 2017

		@@ -1,39 +1,44 @@
		{% set version = "1.3.2" %}
		{% set name = "blosc" %}

		@@ -1,39 +1,44 @@
		{% set version = "1.3.2" %}
		{% set name = "python-blosc" %}

Cesc/update recipe #2

Cesc/update recipe #2

Conversation

FrancescElies commented Nov 2, 2016

conda-forge-linter commented Nov 2, 2016

conda-forge-linter commented Nov 2, 2016

mrocklin commented Nov 2, 2016

jakirkham commented Nov 3, 2016

jakirkham commented Nov 3, 2016

FrancescElies commented Nov 3, 2016

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

danielfrg Nov 3, 2016 • edited Loading

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

jakirkham commented Nov 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielfrg commented Nov 3, 2016

jakirkham commented Nov 3, 2016

jakirkham commented Nov 3, 2016

FrancescElies commented Nov 3, 2016

FrancescElies commented Nov 3, 2016

FrancescElies commented Nov 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msarahan commented Nov 3, 2016

FrancescElies commented Nov 3, 2016

jakirkham 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

jakirkham commented Nov 3, 2016

jakirkham commented Nov 3, 2016

FrancescElies commented Nov 3, 2016

FrancescElies commented Nov 4, 2016

danielfrg commented Nov 4, 2016

FrancescElies commented Nov 4, 2016

jakirkham commented Nov 4, 2016

FrancescElies commented Nov 4, 2016

FrancescElies commented Nov 4, 2016

mrocklin commented Nov 30, 2016

conda-forge-linter commented Dec 1, 2016

FrancescElies commented Dec 1, 2016 • edited Loading

FrancescElies commented Dec 1, 2016

FrancescElies commented Jan 31, 2017

FrancescElies commented Feb 7, 2017

danielfrg Nov 3, 2016 •

edited

Loading

FrancescElies commented Dec 1, 2016 •

edited

Loading