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

Dependency on dill #1

Open
cdeil opened this issue Jul 17, 2015 · 6 comments
Open

Dependency on dill #1

cdeil opened this issue Jul 17, 2015 · 6 comments

Comments

@cdeil
Copy link
Contributor

cdeil commented Jul 17, 2015

@fred3m – Thanks for making this package!

I get this:

astromatic_wrapper/utils/tests/test_pipeline.py:11: in <module>
>   import dill
E   ImportError: No module named 'dill'

dill is not listed here:
http://astromatic-wrapper.readthedocs.org/en/latest/install.html#requirements

I've had several problems installing dill in the past, so if you can avoid the dependency somehow, I think you should.

Also here six is listed as a dependency:
http://astromatic-wrapper.readthedocs.org/en/latest/install.html#requirements

Astropy bundles six, so you can avoid the extra dependency and having to explain about it to users by using

from astropy.extern import six
@fred3m
Copy link
Owner

fred3m commented Jul 17, 2015

Thanks for letting me know. I actually started using six from astropy in the code I just forgot to update the documents.

As for dill, it should be listed in the docs as an optional requirement. Everything runs fine without dill but I use it to serialize the pipeline before it runs each step so that if something happens during execution a user can reload the pipeline, edit any parameters that might have made it crash, and restart it where they left off. I found that pickle wasn't sufficient for some of my pipelines and I assume it will be the same for others.

If this becomes too much of a problem I can have the code try to use pickle if dill isn't present but since dill is included in conda now as well as available on pypi I'm hoping it is more stable for everyone these days.

@fred3m
Copy link
Owner

fred3m commented Jul 17, 2015

Ok, I just updated the docs to remove the six dependency and added dill as an optional requirement. I also moved dill into a function in the test module so that it is not a required dependency. You should be able to install without it now. Let me know if you have any more problems.

@cdeil
Copy link
Contributor Author

cdeil commented Jul 19, 2015

I'm still getting this fail because I don't have dill:

_______________________________________________________ TestPipeline.test_run_advanced ________________________________________________________

self = <astromatic_wrapper.utils.tests.test_pipeline.TestPipeline object at 0x106bf85c0>
tmpdir = local('/private/var/folders/sb/4qv5j4m90pz1rw7m70rj1b1r0000gn/T/pytest-4/test_run_advanced0')

    def test_run_advanced(self, tmpdir):
>       import dill
E       ImportError: No module named 'dill'

astromatic_wrapper/utils/tests/test_pipeline.py:154: ImportError

The recommended way to skip tests if optional dependencies are missing:
http://astropy.readthedocs.org/en/latest/development/testguide.html#tests-requiring-optional-dependencies

You have to add this here:
https://github.com/fred3m/astromatic_wrapper/blob/master/astromatic_wrapper/utils/tests/test_pipeline.py#L153

Here's an example how to configure the travis-ci test matrix to have a build without the optional dependencies ... this is pretty useful:
https://github.com/astrofrog/reproject/blob/master/.travis.yml#L18

@fred3m
Copy link
Owner

fred3m commented Jul 20, 2015

Sorry, I didn't realize that the test functions were run when installing a
package. I'll fix this today.

-Fred
On Jul 19, 2015 5:09 PM, "Christoph Deil" [email protected] wrote:

I'm still getting this fail because I don't have dill:

_______________________________________________________ TestPipeline.test_run_advanced ________________________________________________________

self = <astromatic_wrapper.utils.tests.test_pipeline.TestPipeline object at 0x106bf85c0>
tmpdir = local('/private/var/folders/sb/4qv5j4m90pz1rw7m70rj1b1r0000gn/T/pytest-4/test_run_advanced0')

def test_run_advanced(self, tmpdir):
  import dill

E ImportError: No module named 'dill'

astromatic_wrapper/utils/tests/test_pipeline.py:154: ImportError

The recommended way to skip tests if optional dependencies are missing:

http://astropy.readthedocs.org/en/latest/development/testguide.html#tests-requiring-optional-dependencies

You have to add this here:

https://github.com/fred3m/astromatic_wrapper/blob/master/astromatic_wrapper/utils/tests/test_pipeline.py#L153

Here's an example how to configure the travis-ci test matrix to have a
build without the optional dependencies ... this is pretty useful:
https://github.com/astrofrog/reproject/blob/master/.travis.yml#L18


Reply to this email directly or view it on GitHub
#1 (comment)
.

@cdeil
Copy link
Contributor Author

cdeil commented Jul 20, 2015

I didn't realize that the test functions were run when installing a
package.

They aren’t.
I was executing python setup.py test.

@fred3m
Copy link
Owner

fred3m commented Jul 20, 2015

Gotcha. Thanks for the help, I've added this to my toolkit but decided to do it a bit differently. I feel like serialization is an important part of the pipeline so I modified the code slightly so that if dill fails to import, the Pipeline tries to save itself using pickle (and I fixed the testing module accordingly) and warns the user if it fails.

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

No branches or pull requests

2 participants