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

Adds Test for TRACEMap #2504

Merged
merged 7 commits into from Mar 8, 2018
Merged

Adds Test for TRACEMap #2504

merged 7 commits into from Mar 8, 2018

Conversation

yashkgp
Copy link
Contributor

@yashkgp yashkgp commented Mar 5, 2018

Fixes #1891

@pep8speaks
Copy link

pep8speaks commented Mar 5, 2018

Hello @yashkgp! Thanks for updating the PR.

Line 12:1: E302 expected 2 blank lines, found 1
Line 18:1: E302 expected 2 blank lines, found 1
Line 22:1: E302 expected 2 blank lines, found 1
Line 28:1: E302 expected 2 blank lines, found 1
Line 32:1: E302 expected 2 blank lines, found 1

Comment last updated on March 07, 2018 at 18:30 Hours UTC


def test_measurement():
"""Tests the measurement property of the TRACEMap object."""
assert int(trace.measurement) == 171
Copy link
Member

Choose a reason for hiding this comment

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

does measurement not return a Quantity object? If I remember correctly it should. Is waveunit not set correctly for TRACE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, @Cadair waveunit is been set to None, and that's why measurement is returning a str.


path = sunpy.data.test.rootdir
fitspath = glob.glob(os.path.join(path, "tsi20010130_025823_a2.fits"))
trace = Map(fitspath)
Copy link
Member

Choose a reason for hiding this comment

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

this should be a pytest fixture

@codecov
Copy link

codecov bot commented Mar 5, 2018

Codecov Report

Merging #2504 into master will increase coverage by 6.3%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2504     +/-   ##
=========================================
+ Coverage   76.34%   82.65%   +6.3%     
=========================================
  Files         165      165             
  Lines       12717    12717             
=========================================
+ Hits         9709    10511    +802     
+ Misses       3008     2206    -802
Impacted Files Coverage Δ
sunpy/extern/bundled/six.py 71.93% <0%> (+0.25%) ⬆️
sunpy/util/cond_dispatch.py 85.96% <0%> (+0.87%) ⬆️
sunpy/net/fido_factory.py 94.77% <0%> (+1.3%) ⬆️
sunpy/spectra/spectrogram.py 67.39% <0%> (+1.58%) ⬆️
sunpy/net/dataretriever/client.py 93.33% <0%> (+2%) ⬆️
sunpy/net/vso/attrs.py 85.08% <0%> (+2.2%) ⬆️
sunpy/util/progressbar.py 97.05% <0%> (+2.94%) ⬆️
sunpy/database/database.py 95.34% <0%> (+3.56%) ⬆️
sunpy/net/download.py 85.61% <0%> (+3.59%) ⬆️
sunpy/net/dataretriever/sources/norh.py 100% <0%> (+4.08%) ⬆️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5924b0b...a16fbed. Read the comment docs.

@Cadair
Copy link
Member

Cadair commented Mar 5, 2018

also @yashkgp can you merge the sunpy master branch into this?

import sunpy.data.test

path = sunpy.data.test.rootdir
fitspath = glob.glob(os.path.join(path, "tsi20010130_025823_a2.fits"))
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to use glob in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No . I think I forgot to remove it , while experimenting with the code .

@@ -0,0 +1,35 @@
import os
import glob
Copy link
Member

Choose a reason for hiding this comment

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

You forgot to remove the import

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry , My bad .

@Cadair Cadair merged commit 0b9cc58 into sunpy:master Mar 8, 2018
@yashkgp yashkgp deleted the test_trace branch March 19, 2018 15:38
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

Successfully merging this pull request may close these issues.

None yet

4 participants