tests for IRISRasterClass #7
Conversation
|
Hi @abit2. Thanks for another PR. I'll take a look at this today. Just a quick request. When making PRs, can you give a brief explanation of what the PR does/changes and why? Not just a ping. Just makes it easier and quicker for me to figure out what's going on. Feel free to include a ping as well though ;) Cheers. |
irispy/tests/test_spectrograph.py
Outdated
| hdulist = fits.open(os.path.join(testpath, 'iris_l2_20170222_153635_3690215148_raster_t000_r00000.fits')) | ||
| for key, value in six.iteritems(wcs_l): | ||
| for key_, value_ in wcs_l[key].iteritems(): |
There was a problem hiding this comment.
Inconsistent indentation here.
irispy/tests/test_spectrograph.py
Outdated
| data3[hdulist[3].data == -200.] = np.nan | ||
|
|
||
| spectral_windows = iris_l2_test_raster.data.keys() |
There was a problem hiding this comment.
I think it would be better to get the spectral window names from iris_l2_test_raster.spectral_windows.name. This means we aren't locked into iris_l2_test_raster.data being a dict.
| return get_pkg_data_filename(filename, package="irispy.data.test", **kwargs) | ||
|
|
||
|
|
||
| file_list = glob.glob(os.path.join(rootdir, '*.[!p]*')) |
There was a problem hiding this comment.
Why is this variable defined? Is it used anywhere?
There was a problem hiding this comment.
not yet but it give the list of all the files in the directory
There was a problem hiding this comment.
What's the intention of including it in this PR?
There was a problem hiding this comment.
it can help in giving the list of files when using the terminal
There was a problem hiding this comment.
Unless there's a clear reason we want it in, I think we should omit it in this PR. If we find we need it for something later we can always add it in then.
|
There's a FileNotFoundError when Travis runs the the tests: |
|
@DanRyanIrish it is passing if you run the test with |
|
That makes me think the way you've defined the filepath isn't generally applicable. |
|
@DanRyanIrish |
|
Hmmm. We might have to bring @Cadair to explain this then. Or point us to who can explain it. |
|
It all passes! Good work @abit2 👍 |
ping @DanRyanIrish