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 recipe for Spyder #477

Merged
merged 13 commits into from
May 3, 2016
Merged

Add recipe for Spyder #477

merged 13 commits into from
May 3, 2016

Conversation

ccordoba12
Copy link
Contributor

@msarahan, here it is the PR for Spyder you asked for :-)

@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/spyder) and found some lint.

Here's what I've got...

For recipes/spyder:

  • The top level meta keys are in an unexpected order. Expecting ['package', 'source', 'build', 'requirements', 'test', 'app', 'about'].
  • The recipe could do with some maintainers listed in the "extra/recipe-maintainers" section.
  • Selectors are suggested to take a <two spaces>#<one space>[<expression>] form.

environment thanks to the support of IPython and popular Python libraries
such as NumPy, SciPy, or matplotlib.
doc_url: http://pythonhosted.org/spyder/
dev_url: https://github.com/spyder-ide/spyder
Copy link
Member

Choose a reason for hiding this comment

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

If that is the same as home maybe we don't really need it.

Copy link
Member

@goanpeca goanpeca Apr 29, 2016

Choose a reason for hiding this comment

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

@ccordoba12 there is a mistake here

home should be: http://spyder-ide.org

The home web page is under construction so it is pointing to the github repo in the meantime

Copy link
Contributor Author

@ccordoba12 ccordoba12 May 1, 2016

Choose a reason for hiding this comment

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

I changed the home field to reflect this :-)

@jakirkham
Copy link
Member

jakirkham commented Apr 28, 2016

Is the logo not in the repo? If it is, maybe we should use it from there.


source:
fn: spyder-2.3.9.zip
url: https://pypi.python.org/packages/fb/37/09b789dbe321894afe1657f11f0026a5ea79987f1c30176b92ee648b3faa/spyder-2.3.9.zip
Copy link
Member

Choose a reason for hiding this comment

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

Switch to https://pypi.io/packages/source/s/spyder/spyder-2.3.9.zip.

@jakirkham
Copy link
Member

With all the about stuff. I'm ok with whatever you want to do really, @ccordoba12. I'm not going to argue over it. Generally, we try to keep this stuff lean, but if you think it adds value that's fine by me.

@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/spyder) and found it was in an excellent condition.


extra:
recipe-maintainers:
ccordoba12
Copy link
Member

Choose a reason for hiding this comment

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

So, this needs to be a list.

@ccordoba12
Copy link
Contributor Author

This is failing because:

  1. CircleCI is not using the latest conda-build (but 1.18.2) and so description is not a valid field in that version. Even after commenting that, I found that Spyder can't be tested because nor HOME or USER are defined in the Docker image. Can't that be changed?
  2. Travis fails because of an error in conda-build 1.20.1 and Python 2.7. I'll see if I can submit a PR to fix it :-)

@jakirkham
Copy link
Member

jakirkham commented Apr 29, 2016

Travis fails because of an error in conda-build 1.20.1 and Python 2.7. I'll see if I can submit a PR to fix it :-)

This one should already be fixed by this PR ( conda/conda-build#892 ), but haven't had time to test it. Also, is not released.

@jakirkham
Copy link
Member

CircleCI is not using the latest conda-build (but 1.18.2) and so description is not a valid field in that version. Even after commenting that, I found that Spyder can't be tested because nor HOME or USER are defined in the Docker image. Can't that be changed?

Should be updated I think. Unless it is getting pinned somewhere else.

@jakirkham
Copy link
Member

I think you are on a really old commit, @ccordoba12. If you rebase or merge with current master, I expect all of this will go away.

@jakirkham
Copy link
Member

Yeah, actually, I checked. That is the cause. Just merge with conda-forge/master. We have either fixed or worked around all of these problems.

@jakirkham
Copy link
Member

Added this PR ( ccordoba12#1 ) to merge your branch with current master. That should straighten out these remaining issues.

@jakirkham
Copy link
Member

Looks like there is some encoding issue in the docker build.

+ spyder -h
Traceback (most recent call last):
  File "/opt/conda/envs/_test/bin/spyder", line 2, in <module>
    from spyderlib import start_app
  File "/opt/conda/envs/_test/lib/python2.7/site-packages/spyderlib/start_app.py", line 12, in <module>
    from spyderlib.baseconfig import get_conf_path, running_in_mac_app
  File "/opt/conda/envs/_test/lib/python2.7/site-packages/spyderlib/baseconfig.py", line 188, in <module>
    from spyderlib.otherplugins import PLUGIN_PATH
  File "/opt/conda/envs/_test/lib/python2.7/site-packages/spyderlib/otherplugins.py", line 17, in <module>
    from spyderlib.utils import programs
  File "/opt/conda/envs/_test/lib/python2.7/site-packages/spyderlib/utils/programs.py", line 29, in <module>
    username = encoding.to_unicode_from_fs(os.environ.get('USER'))
  File "/opt/conda/envs/_test/lib/python2.7/site-packages/spyderlib/utils/encoding.py", line 62, in to_unicode_from_fs
    string = to_text_string(string.toUtf8(), 'utf-8')
AttributeError: 'NoneType' object has no attribute 'toUtf8'

@msarahan
Copy link
Member

No, that is because the USER env var is not set.

@jakirkham
Copy link
Member

Ah, right. Hmm...should that case be handled by Spyder?

I guess we can just set a user in our docker image. At a bare minimum we can set that to root for now.

We probably should be adding a non-root user for security reasons. Though this image is not necessarily intended for any use beyond building on CIs or local debugging.

@jakirkham
Copy link
Member

Sorry guys. I thought I had pinned conda-build correctly to avoid the Mac issue here, but I did not. Correcting it in this PR ( #488 ). Will merge as soon as it passes and restart CI here.

@jakirkham
Copy link
Member

That PR is merged and CI has been restarted.

@jakirkham
Copy link
Member

That fixed the Travis CI failure.


test:
commands:
- USER=test spyder -h # [unix]
Copy link
Member

@jakirkham jakirkham May 1, 2016

Choose a reason for hiding this comment

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

Kind of hacky, but I understand why we need this here and we may need it for a bit. Really we should change the docker image to be a little more realistic on this point. I've added an issue ( conda-forge/docker-images#6 ) for changing the docker image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know that this a bit hacky, but useful for the moment :-)

Copy link
Member

Choose a reason for hiding this comment

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

It's fine. We should be doing a better job so that you don't have to do this. Just felt like I should add a note to explain to myself and others why we need to change the docker image to fix this.

@jakirkham
Copy link
Member

CI is passing now. However, there a few issues mentioned above that I would like us to take care of before merging.

@ccordoba12
Copy link
Contributor Author

ccordoba12 commented May 1, 2016

@jakirkham, I think this is ready :-)


Is the logo not in the repo? If it is, maybe we should use it from there.

The logo.png file is necessary for Anaconda Navigator.


I also left all new fields in the about section for Navigator.

@jakirkham
Copy link
Member

Is the logo not in the repo? If it is, maybe we should use it from there.

The logo.png file is necessary for Anaconda Navigator.

I understand. I was just hoping that we could grab it from the zip on PyPI instead of having it bundled with the recipe. Would that be possible?

@ccordoba12
Copy link
Contributor Author

I was just hoping that we could grab it from the zip on PyPI instead of having it bundled with the recipe. Would that be possible?

I wouldn't like to explore this here but in the feedstock. Besides, it's a very small file :-)

@ocefpaf
Copy link
Member

ocefpaf commented May 3, 2016

@ccordoba12 this LGTM. I am merging and any renaming issue can be addressed in the feedstock.

Awesome contribution! I will be using it soon to teach my tutorial at SciPyLA 2016 😉

@ocefpaf ocefpaf merged commit 50c6d1b into conda-forge:master May 3, 2016
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.

None yet

7 participants