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

imagepy #5450

Merged
merged 7 commits into from
Mar 23, 2018
Merged

imagepy #5450

merged 7 commits into from
Mar 23, 2018

Conversation

yxdragon
Copy link
Contributor

No description provided.

@yxdragon
Copy link
Contributor Author

@jbcrail sknw has been merged! That is imagepy, which need wxpython, I did not know if it can be solved!

@mariusvniekerk
Copy link
Member

So the easiest way to get circle to run your tests after uostream things are merged is to just close and reopen this pull request. Barring that make an empty commit with git commit --allow-empty

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

@jakirkham
Copy link
Member

Took the liberty of merging this with conda-forge/master and pushed a commit to your branch to do just that, @yxdragon. Hope that is ok.

Appears to have addressed whatever CI issues and lingering recipe issues that were here.

home: https://github.com/Image-Py/imagepy
license: BSD 3-clause
license_family: BSD
license_file: ''
Copy link
Member

Choose a reason for hiding this comment

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

Added PR ( Image-Py/imagepy#16 ) upstream, which should ensure the license file can be included in packages in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jakirkham need I re upload to pypi first?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, so now that you've merged that PR the next upload to PyPI will have the license file in it. If you want to do a patch release now that includes that, then you could update the recipe to use that version and set license_file: LICENSE. Otherwise you could just remember to do it for the next release.

Copy link
Member

Choose a reason for hiding this comment

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

Or could copy the license file over here for now, in which case it can be dropped from the recipe on the next update. Anyways lots of options to choose from here, feel free to pick what works best for you.

license_family: BSD
license_file: ''
summary: interactive python image-processing plugin framework
description: Image process framework based on plugin like imagej, it is esay to glue with scipy.ndimage, scikit-image, opencv, simpleitk, mayavi...and any libraries based on numpy
Copy link
Member

Choose a reason for hiding this comment

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

Would suggest wrapping the description. Here is a good example.


build:
number: 0
script: python setup.py install --single-version-externally-managed --record=record.txt
Copy link
Member

@jakirkham jakirkham Mar 22, 2018

Choose a reason for hiding this comment

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

Apologies. The example is a bit outdated. We should fix that. Currently we encourage using pip for the install.

requirements:
build:
- python
- setuptools
Copy link
Member

Choose a reason for hiding this comment

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

Also would change setuptools to pip.

sha256: {{ hash_value }}

build:
number: 0
Copy link
Member

Choose a reason for hiding this comment

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

Is this a pure Python package? If so, would take a look at using noarch: python. This would build one universal Python package that users could install on Python 2/3 on all platforms.

Copy link
Contributor Author

@yxdragon yxdragon Mar 22, 2018

Choose a reason for hiding this comment

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

yes, pure python package, (with numba, pure?)
@jakirkham I think it is the matter of wxpython, here: https://wxpython.org/pages/downloads/

"Because of the differences between Linux distributions (mainly different versions of the core libraries installed by default, but also platform architecture and etc.) it is not possible to host binary wheel files for Linux on PyPI..."

wxpython.org say pypi not work on linux, I do not know if conda can solve it? and is there any wx app on conda? I want to have a look!

Copy link
Contributor

@djsutherland djsutherland Mar 22, 2018

Choose a reason for hiding this comment

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

@yxdragon conda-forge has a wxpython package: https://github.com/conda-forge/wxpython-feedstock. It should work fine.

If imagepy only has Python code in it, it can be noarch: python, even if it uses Numba / dependencies that aren't Python – it's just a question of whether imagepy itself installs any different files for different architectures, e.g. because it has to compile code.

Copy link
Member

Choose a reason for hiding this comment

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

Run time compilation via Numba still counts as pure Python. We are only concerned with compilation that occurs at build time and it sounds like imagepy doesn't do that. So imagepy counts as pure Python.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dougalsutherland
that is my CI build log, libgtk-x11-2.0, so I think wx does not work.

from wx.core import *
File "/home/conda/staged-recipes/build_artifacts/imagepy_1521691888499/_t_env/lib/python2.7/site-packages/wx/core.py", line 12, in
from ._core import *
ImportError: libgtk-x11-2.0.so.0: cannot open shared object file: No such file or directory

Copy link
Member

Choose a reason for hiding this comment

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

Ack. Sorry about that.

So we are not as cleanly separated from the OS as we would like. In some cases, particularly GUI packages, we rely on system backed things like Gtk and X11 to be provided for us. Ideally we would package things like X11 (we have done a lot of work in that direction) and Gtk, which we are working on in PR ( #3920 ). In cases where we rely on system packages, we notate this in a yum_requirements.txt file in the recipe like this one for wxpython.

Would just copy wxpython's yum_requirements.txt file and place it in your recipe here. This should install the needed Gtk and X11 libraries behind the scenes. In practice users will typically already have X11 and Gtk as they will be running on a Linux Desktop as opposed to a Docker image (which is where we are building this). In the future, we plan to do a better job of packaging these dependencies outright.

If you have any questions about this, please feel free to ask. :)

Copy link
Member

Choose a reason for hiding this comment

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

Went ahead and added yum_requirements.txt, which should fix this. Hope that is ok.

@jakirkham
Copy link
Member

Overall this looks really good, @yxdragon. Thanks for adding this.

Added a few comments about, but they shouldn't be too difficult. Please let us know if you have any questions.

Thoughts from @conda-forge/staged-recipes?

@yxdragon yxdragon closed this Mar 22, 2018
@yxdragon yxdragon reopened this Mar 22, 2018
@conda-forge-linter
Copy link

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

I was trying to look for recipes to lint for you, but couldn't find any.
Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@yxdragon yxdragon closed this Mar 22, 2018
@yxdragon yxdragon reopened this Mar 22, 2018
@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/imagepy) and found it was in an excellent condition.

@yxdragon
Copy link
Contributor Author

@jakirkham I did not know conda is so powerful! and we can also add a apt-get requirements?

@yxdragon
Copy link
Contributor Author

@jakirkham 17 minutes has passed, I click the build detail, and I saw a tag "not runing", is it right?

@djsutherland
Copy link
Contributor

@yxdragon Sometimes the CIs get a little busy and it takes a while. Patience. :)

Also: yum_requirements.txt is only used for building on the CIs – it has nothing to do with what users see. So apt-get requirements wouldn't do anything, and the package will still unfortunately break if the user doesn't have gtk / x11 installed.

@yxdragon
Copy link
Contributor Author

@dougalsutherland so the "conda install wxpython" does not work on linux?

@djsutherland
Copy link
Contributor

It does, as long as the user has gtk and x11 installed themselves. They usually will if they're on a desktop machine.

@yxdragon
Copy link
Contributor Author

OK, I would try it after ImagePy is merged.

@yxdragon
Copy link
Contributor Author

The CI is too busy, I would have a look tomorrow, you (members of canda-forge org) are so friendly and responsible, Thanks!

@jakirkham
Copy link
Member

FWIW we are interested in packaging Gtk and it is in the last stages. There already is a cross platform set of X11 packages, but we are working out the details on how best to use them. So I think in the future we will just use these packages and not yum, which should be a bit cleaner.

summary: interactive python image-processing plugin framework
description: |
Image process framework based on plugin like imagej.
It is esay to glue with any libraries based on numpy.
Copy link
Contributor

Choose a reason for hiding this comment

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

esay -> easy

@djsutherland
Copy link
Contributor

We don't need to wait for CIs to run again for this one-character change to the description. 😉

Merging; thanks @yxdragon!

@djsutherland djsutherland merged commit 8b7a6b8 into conda-forge:master Mar 23, 2018
@yxdragon
Copy link
Contributor Author

@dougalsutherland thanks, I would try to install on py2/3 under win, mac, linux, If there are some question, may need your help! by the way, If I want to update, I will upload to pypi a new version, then what should I do to update conda-forge?

@djsutherland
Copy link
Contributor

djsutherland commented Mar 23, 2018

It'll take a moment for the build to show up on anaconda.org, but the home for the imagepy recipe is now https://github.com/conda-forge/imagepy-feedstock (which you should have gotten an automatic invitation to).

When a new version comes out, you can either (a) wait for the auto-tick bot to send a PR updating the version (< a day after it's up on PyPI; example: conda-forge/tmux-feedstock#6), or (b) send a PR yourself. (Do the PR from your own fork, rather than a branch on the conda-forge feedstock: branches automatically get uploaded also!) In the PR, all you should have to do is bump the version variable, update the sha256 (which you can get from https://pypi.org/project/imagepy/#files), and update any dependencies if they changed. The bot PR will do the first two parts for you, but you'll still have to edit it if any dependencies or anything else about the recipe changed.

@jakirkham
Copy link
Member

Thanks all. Thanks @yxdragon for submitting. Feel free to submit plugin packages if you like as well.

@yxdragon
Copy link
Contributor Author

@jakirkham ImagePy's plugins is managed in menus, tools, widgets folder, and parse these plugins to generate UI, I will think about how to manage imagepy's plugin more elegantly, and more convenient to publish, search and download, maybe a package named ipy_xxx, put the plugins file in, and with a catlog file to show which parent menus this plugin belong, Is there a interface I can search conda-forge's packages and list those start with 'ipy_'?

@yxdragon
Copy link
Contributor Author

@jakirkham @dougalsutherland
I try run on Ubuntu 16.4, but got some error, (wxpython)
......
ImportError: libiconv.so.2: cannot open shared object file: No such file or directory

see https://wxpython.org/pages/downloads/, we must download the whl according to the linux core and version. I think conda can solve this promblem, but if should I open a issue to wxpython orgnization?
Next, I want to build a web based ImagePy. qt/wx is too heavy, difficult to install...

@jakirkham
Copy link
Member

Do you already gtk2 and webkitgtk installed from apt-get? gtk2 should pull that in as a dependency.

@jakirkham
Copy link
Member

Ah ok. Thanks for the info about the plugins.

Might want to chat with Jupyter and/or Spyder developers who have thought more about plugin architecture. Expect they will have some good advice that would be useful as you look into this more.

cc @minrk @ccordoba12

@minrk
Copy link
Member

minrk commented Mar 26, 2018

We've explored afew extension mechanisms over the years in IPython/Jupyter:

  1. placing files in $PREFIX/share/jupyter for jupyter extensions. This works for us because Jupyter extensions may not be Python specific (often they are purely javascript), but has management downsides since things may not be pure Python packages
  2. %load_ext <pythonmodule> in IPython, calls pythonmodule.load_ipython_extension(self). This works well, but has no mechanism for discovery.
  3. using setuptools entrypoints, e.g. for registering nbconvert exporters

I think for a Python package, entrypoints are the way to go because they make discovery really simple and efficient. Just make sure to use the entrypoints package for runtime discovery instead of pkg_resources, since pkg_resources does wildly inefficient imports on load.

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.

7 participants