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 SunPy, Glymur and wcsaxes #605

Merged
merged 28 commits into from
May 17, 2016
Merged

Add SunPy, Glymur and wcsaxes #605

merged 28 commits into from
May 17, 2016

Conversation

Cadair
Copy link
Member

@Cadair Cadair commented May 13, 2016

I think all the other deps are already in conda-force or defaults, so I think this is all I need!

@conda-forge-linter
Copy link

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 (recipes/glymur, recipes/sunpy) and found some lint.

Here's what I've got...

For recipes/sunpy:

  • Selectors are suggested to take a <two spaces>#<one space>[<expression>] form.

@astrofrog
Copy link
Contributor

@Cadair you don't need the separate build scripts in your case - you can just add the following line in your YAML file:

https://github.com/astrofrog/staged-recipes/blob/f7eae747ab79eaaf2cd02b649a78d87883a0b2f5/recipes/glueviz/meta.yaml#L14

@astrofrog
Copy link
Contributor

And you can add the extra flags for Sunpy (but you don't need separate build scripts)

@Cadair
Copy link
Member Author

Cadair commented May 13, 2016

handy! you know how to skip python 3 builds?!

@conda-forge-linter
Copy link

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 (recipes/glymur, recipes/sunpy) and found it was in an excellent condition.

@astrofrog
Copy link
Contributor

astrofrog commented May 13, 2016

Someone more knowledgeable than me can comment on it, but you might be able to add something like

skip: True  # [py>27]

in the build section (total guess). The documentation is pretty sparse:

http://conda.pydata.org/docs/building/meta-yaml.html#skipping-builds

@Cadair
Copy link
Member Author

Cadair commented May 13, 2016

ping @quintusdias you might find this interesting!

@ocefpaf
Copy link
Member

ocefpaf commented May 13, 2016

skip: True # [py>27] will work, but I prefer skip: True # [py3k]

Thanks @Cadair for the PR and @astrofrog for the review!


build:
number: 0
script: python setup.py install
Copy link
Member

Choose a reason for hiding this comment

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

When setuptools is available you can use

script: python setup.py install --single-version-externally-managed --record record.txt

That makes the package more "external packager manager friendly."

Copy link
Member Author

Choose a reason for hiding this comment

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

@astrofrog you got any idea how they interact with astropy_helpers?

Copy link
Contributor

@astrofrog astrofrog May 13, 2016

Choose a reason for hiding this comment

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

No, but make sure you add --offline --no-git for the sunpy build, and we'll find out! (sorry just saw you already have those - so just try combining with the above flags)

- setuptools
- numpy >=1.7.0
- lxml >=2.3.2
- mock
Copy link
Member

Choose a reason for hiding this comment

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

Is mock a build and run dependency? Or is it just for tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's probably only a build dep.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just for tests. I should remove that...

On Fri, May 13, 2016 at 11:59 AM, Filipe notifications@github.com wrote:

In recipes/glymur/meta.yaml
#605 (comment)
:

+build:

  • number: 0
  • script: python setup.py install --single-version-externally-managed --record record.txt
  • entry_points:
    • jp2dump=glymur.command_line:main

+requirements:

  • build:
    • python
    • setuptools
    • numpy >=1.7.0
    • lxml >=2.3.2
    • mock

Is mock a build and run dependency? Or is it just for tests?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/conda-forge/staged-recipes/pull/605/files/d4229db6df4a8530c5778ff1fba2a2fc872d612a#r63207713

John Evans

Copy link
Member Author

Choose a reason for hiding this comment

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

it needs it to build, but I have now removed it from the runtime deps.

@conda-forge-linter
Copy link

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 (recipes/glymur, recipes/sunpy, recipes/wcsaxes) and found some lint.

Here's what I've got...

For recipes/wcsaxes:

  • The summary item is expected in the about section.

@conda-forge-linter
Copy link

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 (recipes/glymur, recipes/sunpy, recipes/wcsaxes) and found it was in an excellent condition.

test:
# Python imports
imports:
- wcsaxes
Copy link
Contributor

Choose a reason for hiding this comment

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

You could technically also run the test suite with wcsaxes.test(), it will skip the image tests anyway

Copy link
Member Author

Choose a reason for hiding this comment

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

is there a better way than:

test:
  commands:
    - python -c "import wcsaxes; wcsaxes.test()"

??

test:
# Python imports
imports:
- wcsaxestest:
Copy link
Contributor

Choose a reason for hiding this comment

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

@Cadair - Something went wrong here?

@Cadair
Copy link
Member Author

Cadair commented May 15, 2016

@kmuehlbauer Thanks for the suggestion!

This is a work around to get the 0.6.4 SunPy release built, it will need
fixing for SunPy 0.7
@conda-forge-linter
Copy link

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 (recipes/glymur, recipes/sunpy, recipes/wcsaxes) and found some lint.

Here's what I've got...

For recipes/glymur:

  • Failed to even lint the recipe (might be a conda-smithy bug) 😢

@conda-forge-linter
Copy link

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 (recipes/glymur, recipes/sunpy, recipes/wcsaxes) and found it was in an excellent condition.

@Cadair
Copy link
Member Author

Cadair commented May 16, 2016

I think this is ready to go, at least for now.

@astrofrog
Copy link
Contributor

@ocefpaf @kmuehlbauer - I think this is ready to merge?

@kmuehlbauer
Copy link
Contributor

@ocefpaf Can you have a look onto the mkl pinning? I have no idea, if this has side effects. Unfortunately there is no solution from continuum so far.

@Cadair
Copy link
Member Author

Cadair commented May 17, 2016

Once the bug is fixed I will remove the mkl pin, unless someone else has another solution.

build:
- python
- setuptools
- numpy >=1.7.0
Copy link
Member

Choose a reason for hiding this comment

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

In theory is not need as we only build numpy > 1.9 already, but I guess it does not hurt.

Copy link
Member Author

Choose a reason for hiding this comment

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

i suppose the only use would be if someone has some custom packages coming from somewhere else. It's probably worth keeping.

@ocefpaf
Copy link
Member

ocefpaf commented May 17, 2016

@Cadair this is good to merge. I made a few minor comments, but I can merge as soon as you give me the go with or without them.

@Cadair
Copy link
Member Author

Cadair commented May 17, 2016

@ocefpaf Small fixes pushed, I am ready when you are.

@Cadair
Copy link
Member Author

Cadair commented May 17, 2016

Wooo the build passed!

@ocefpaf ocefpaf merged commit 13c9347 into conda-forge:master May 17, 2016
@Cadair
Copy link
Member Author

Cadair commented May 17, 2016

Thanks very much @ocefpaf

@ocefpaf
Copy link
Member

ocefpaf commented May 17, 2016

Thanks for the recipes @Cadair. Let me know if you have any questions about conda-forge.

@Cadair
Copy link
Member Author

Cadair commented May 17, 2016

@ocefpaf the only remaing build question is what caused this: https://ci.appveyor.com/project/conda-forge/staged-recipes/build/1.0.2359/job/y263lbubo8aoxw18#L675

I have worked around it for now.

@ocefpaf
Copy link
Member

ocefpaf commented May 17, 2016

@Cadair open an issue for that in the feedstock and we can take a look at that. (I am in a conference right now with limited access to internet, so it might take a while for me to get back to that.)

@Cadair
Copy link
Member Author

Cadair commented May 17, 2016

@ocefpaf cool.

@jakirkham
Copy link
Member

Could someone please open a bug for the mkl issue here at staged-recipes and cc a bunch of people that have been having this issue? I haven't followed what happened or had a chance to explore, but it would be nice to get the bird's eye view 🐦. Then we can start talking with Continuum about what is going on here. Thanks.

@jakirkham
Copy link
Member

Could I also just install nomkl under windows for the build? Or would that then propagate into the dependencies for the package?

There is no nomkl on Windows, @Cadair. This is because everything was always built with MKL on Windows even before MKL was released to defaults. See the discussion in this PR ( conda-archive/conda-recipes#586 ). We may try getting OpenBLAS working here for Windows ( conda-forge/openblas-feedstock#2 ). Though that problem is a bit involved.

@Cadair
Copy link
Member Author

Cadair commented May 17, 2016

thanks @jakirkham I noticed the lack of nomkl when I tried it to shut the build up. At least that means it should only cause the users MKL to be downgraded.

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

Successfully merging this pull request may close these issues.

8 participants