STScI-Citizen-Science / MTPipeline

Pipeline to produce CR rejected, astrodrizzled, png's of HST WFPC2 solar system data.
6 stars 1 forks source link

Test WFPC2 make_output_file_dict #81

Closed acviana closed 10 years ago

acviana commented 10 years ago

Summary

The make_output_file_dict function in imaging_pipeline.py is currently used to generate the output files names for the imaging pipeline. In the interest of following the DRY principle ("Don't Repeat Yourself") this function should also be used to generate the file names for the completeness test we are about to write. This is a good time to move some files around and add some test coverage to the package using what we learned in #79.

Steps

$ git branch completeness-test
$ git status
$ git checkout completeness-test
$ git status
$ git push origin completeness-test
walyssonBarbosa commented 10 years ago

Here's the test I created with only one input file:

def test_make_output_file_dict():
    result = {'file_name': 'u2eu0101t_c0m.fits', 'cr_reject_output': ['u2eu0101t_c0m.fits', 'u2eu0101t_cr_c0m.fits'], 'drizzle_output': ['u2eu0101t_c0m_wide_single_sci.fits', 'u2eu0101t_c0m_center_single_sci.fits', 'u2eu0101t_cr_c0m_wide_single_sci.fits', 'u2eu0101t_cr_c0m_center_single_sci.fits'], 'png_output': ['u2eu0101t_c0m_wide_single_sci_log.png', 'u2eu0101t_c0m_wide_single_sci_median.png', 'u2eu0101t_c0m_center_single_sci_log.png', 'u2eu0101t_c0m_center_single_sci_median.png', 'u2eu0101t_cr_c0m_wide_single_sci_log.png', 'u2eu0101t_cr_c0m_wide_single_sci_median.png', 'u2eu0101t_cr_c0m_center_single_sci_log.png', 'u2eu0101t_cr_c0m_center_single_sci_median.png'], 'drizzle_weight': ['u2eu0101t_c0m_wide_single_wht.fits', 'u2eu0101t_c0m_center_single_wht.fits', 'u2eu0101t_cr_c0m_wide_single_wht.fits', 'u2eu0101t_cr_c0m_center_single_wht.fits']}
    assert make_output_file_dict(result['file_name']) == result

I am not able to test if this is correct. It says that there is no module named test_imaging_pipeline.

walyssonBarbosa commented 10 years ago

I tried to push the new branch but I think I don't have permission to do it.

acviana commented 10 years ago

OK, good start. I reformatted your function to make it comply to PEP8 style per the style guide to make it a little more readable. The big changes are wrapping long lines to 79 characters.

def test_make_output_file_dict():
    result = {'file_name': 'u2eu0101t_c0m.fits', 
              'cr_reject_output': ['u2eu0101t_c0m.fits', 
                                   'u2eu0101t_cr_c0m.fits'], 
              'drizzle_output': ['u2eu0101t_c0m_wide_single_sci.fits', 
                                 'u2eu0101t_c0m_center_single_sci.fits', 
                                 'u2eu0101t_cr_c0m_wide_single_sci.fits', 
                                 'u2eu0101t_cr_c0m_center_single_sci.fits'], 
              'png_output': ['u2eu0101t_c0m_wide_single_sci_log.png', 
                             'u2eu0101t_c0m_wide_single_sci_median.png', 
                             'u2eu0101t_c0m_center_single_sci_log.png', 
                             'u2eu0101t_c0m_center_single_sci_median.png', 
                             'u2eu0101t_cr_c0m_wide_single_sci_log.png', 
                             'u2eu0101t_cr_c0m_wide_single_sci_median.png', 
                             'u2eu0101t_cr_c0m_center_single_sci_log.png', 
                             'u2eu0101t_cr_c0m_center_single_sci_median.png'], 
              'drizzle_weight': ['u2eu0101t_c0m_wide_single_wht.fits', 
                                 'u2eu0101t_c0m_center_single_wht.fits', 
                                 'u2eu0101t_cr_c0m_wide_single_wht.fits', 
                                 'u2eu0101t_cr_c0m_center_single_wht.fits']}
    assert make_output_file_dict(result['file_name']) == result

From there things look pretty good. I like your solution of using the file_name dictionary key in the result dict as the input to make_output_file_dict. I would maybe changed the name of the dictionary though. Result to me implies that it's result of some process, but it's not, it's something you defined. Instead I would suggest something that is more descriptive of it's role as the dictionary of expected values. Maybe expected_output_file_dict. It's a long variable name but it's (in my opinion) completely descriptive.

Let me know when you're back in your office and I'll take a look at the python import and git push errors.

walyssonBarbosa commented 10 years ago
Traceback (most recent call last):
  File "/Users/wbarbosa/virtualenvs/completeness-test/lib/python2.7/site-packages/nose/loader.py", line 414, in loadTestsFromName
    addr.filename, addr.module)
  File "/Users/wbarbosa/virtualenvs/completeness-test/lib/python2.7/site-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/Users/wbarbosa/virtualenvs/completeness-test/lib/python2.7/site-packages/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/Users/wbarbosa/virtualenvs/completeness-test/MTPipeline/mtpipeline/tests/test_imaging_pipeline.py", line 3, in <module>
    from mtpipeline.imaging.imaging_pipeline import make_output_file_dict
  File "/Users/wbarbosa/virtualenvs/completeness-test/MTPipeline/mtpipeline/imaging/imaging_pipeline.py", line 22, in <module>
    from stwcs import updatewcs
ImportError: No module named stwcs
acviana commented 10 years ago

Running pip install stwcs will install the missing module.

For the other missing modules you'll have to move the modules in the imaging package and update the import statements in imaging_pipeline.py to be:

from mtpipeline.imaging.run_cosmics import run_cosmics
from mtpipeline.imaging.run_astrodrizzle import run_astrodrizzle
from mtpipeline.imaging.run_trim import run_trim
from mtpipeline.email_decorator import email_decorator
acviana commented 10 years ago

Please see these notes on how to submit your changes: https://github.com/STScI-Citizen-Science/MTPipeline/wiki/Code-Specs-and-Developer-Workflow#pull-requests

Please let me know if anything is unclear so I can update it.

walyssonBarbosa commented 10 years ago

I had to install the package numpy before installing stwcs. And then I installed scipym stscipython and Pillow. Run:

$ pip install numpy --upgrade
$ pip install stwcs
$ pip install scipy
$ pip install stscipython
$ pip install Pillow

Now the missing module is matplotlib, which I am trying to install.

acviana commented 10 years ago

Are you working in the virtualenv we created yesterday? Because setup.py is supposed to install those dependencies for you: https://github.com/STScI-Citizen-Science/MTPipeline/blob/master/setup.py

walyssonBarbosa commented 10 years ago

I am trying to run python setup.y develop but it's not working.

acviana commented 10 years ago

Meaning it's crashing when you try to run setup.py or you're able to run it but it's still not finding the dependencies?

walyssonBarbosa commented 10 years ago

Yes

walyssonBarbosa commented 10 years ago

Have you given me permission to push what I changed to the repo?

When I run git push origin completeness-test it says:

remote: Permission to STScI-Citizen-Science/MTPipeline.git denied to walyssonBarbosa.
fatal: unable to access 'https://github.com/STScI-Citizen-Science/MTPipeline.git/': The requested URL returned error: 403
acviana commented 10 years ago

Sorry, I just created a new team called MTPipeline-push and added you as a member.

walyssonBarbosa commented 10 years ago

I couldn't find the new team.

Even crashing while running python setup.py develop, I could run test_imaging_pipeline.py. This is the message that it displays:

E
======================================================================
ERROR: Failure: ImportError (dlopen(/Users/wbarbosa/.python-eggs/Pillow-2.2.1-py2.7-macosx-10.6-x86_64.egg-tmp/PIL/_imaging.so, 2): Library not loaded: /Users/Shared/ureka.iraf/ur_work/urp/python/lib/libjpeg.8.dylib
  Referenced from: /Users/wbarbosa/.python-eggs/Pillow-2.2.1-py2.7-macosx-10.6-x86_64.egg-tmp/PIL/_imaging.so
  Reason: image not found)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/wbarbosa/Downloads/Ureka/variants/common/lib/python2.7/site-packages/nose/loader.py", line 414, in loadTestsFromName
    addr.filename, addr.module)
  File "/Users/wbarbosa/Downloads/Ureka/variants/common/lib/python2.7/site-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/Users/wbarbosa/Downloads/Ureka/variants/common/lib/python2.7/site-packages/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/Users/wbarbosa/virtualenvs/completeness-test/MTPipeline/mtpipeline/tests/test_imaging_pipeline.py", line 3, in <module>
from mtpipeline.imaging.imaging_pipeline import make_output_file_dict
  File "/Users/wbarbosa/virtualenvs/completeness-test/MTPipeline/mtpipeline/imaging/imaging_pipeline.py", line 27, in <module>
    from mtpipeline.imaging.run_trim import run_trim
  File "/Users/wbarbosa/virtualenvs/completeness-test/MTPipeline/mtpipeline/imaging/run_trim.py", line 17, in <module>
    from PIL import Image
  File "build/bdist.macosx-10.6-x86_64/egg/PIL/Image.py", line 53, in <module>
  File "build/bdist.macosx-10.6-x86_64/egg/PIL/_imaging.py", line 7, in <module>
  File "build/bdist.macosx-10.6-x86_64/egg/PIL/_imaging.py", line 6, in __bootstrap__
ImportError: dlopen(/Users/wbarbosa/.python-eggs/Pillow-2.2.1-py2.7-macosx-10.6-x86_64.egg-tmp/PIL/_imaging.so, 2): Library not loaded: /Users/Shared/ureka.iraf/ur_work/urp/python/lib/libjpeg.8.dylib
  Referenced from: /Users/wbarbosa/.python-eggs/Pillow-2.2.1-py2.7-macosx-10.6-x86_64.egg-tmp/PIL/_imaging.so
  Reason: image not found

----------------------------------------------------------------------
Ran 1 test in 0.002s

FAILED (errors=1)
acviana commented 10 years ago

Please see the comments I left on your latest commit for your next steps.

acviana commented 10 years ago

Hi Wally, I think we can close this now after your merge. Really we should have been more thorough in the merge message and said "closes issue #XX" in the merge and that would have automatically closed it.