-
-
Notifications
You must be signed in to change notification settings - Fork 616
Updated tests for resampling Generic Map and some documentation in the Sunpy/data/test. #2011
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
Conversation
Hello @prateekiiest! Thanks for updating the PR.
Comment last updated on March 15, 2017 at 13:34 Hours UTC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some documentation for sunpy test database
sunpy/net/vso/vso.py
Outdated
@@ -705,13 +705,17 @@ def download(self, method, url, dw, callback, errback, *args): | |||
|
|||
@staticmethod | |||
def by_provider(response): | |||
"""Returns a dictionary of provider corresponding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be formatted like this, it's a docstring not code.
Something like this would be correct. In general the triple quote marks """
should be on their own lines.
def by_provider(response):
"""
Returns a dictionary of provider corresponding
to records in the response.
"""
OK :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the quotation marks.
sunpy/map/tests/test_mapbase.py
Outdated
@@ -365,7 +365,7 @@ def test_resample_metadata(generic_map, sample_method, new_dimensions): | |||
assert resampled_map.meta['crval2'] == generic_map.center.y.value | |||
for key in generic_map.meta: | |||
if key not in ('cdelt1', 'cdelt2', 'crpix1', 'crpix2', | |||
'crval1', 'crval2'): | |||
'crval1', 'crval2', 'naxis1', 'naxis2'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change here? As I commented in #2010 it should not be needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I had changed these things in my forked repo and I didn't delete the previous changes. As a result, when I am opened a PR for this issue, all the previous changes are being shown with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testmap , naxis1,naxis2 removed from key values
_config.yml
Outdated
@@ -0,0 +1 @@ | |||
theme: jekyll-theme-merlot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errrr, why is this here?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Cadair ,Some patch problems might have occurred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updating in _config.yml in sunpy.github.io issues a file change here due to same patch work.
My bad.
Sorry @Cadair
_config.yml
Outdated
@@ -0,0 +1 @@ | |||
theme: jekyll-theme-merlot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Cadair ,Some patch problems might have occurred.
@@ -0,0 +1 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should not be here at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, its a file in the sunpy.github.io in the website making repository.
You can close this PR.
2 or 3 PRs has been collectively addressed here
I will send separate PRs for the other related issues :)
@@ -705,13 +705,21 @@ def download(self, method, url, dw, callback, errback, *args): | |||
|
|||
@staticmethod | |||
def by_provider(response): | |||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You just opened another PR with this change in, it should only be in one place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I have sent a seperate PR for this. This PR is all jumbled up with other PRs, due to some patch mismatch. You can close this PR.
@Cadair
I have updated the keys for generic map (included naxis1 and naxis2).
This PR is done to clear out the error causing in PR #2010