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

Jump to bottom

Added a show method for base_Client #4309

Merged
merged 2 commits into from Jun 26, 2020
Merged

Added a show method for base_Client #4309

merged 2 commits into from Jun 26, 2020

Conversation

abhijeetmanhas
Copy link
Contributor

Description

Fixes #556
This would enable users to specify the columns they want to see in the Query results for vso, jsoc and DR clients.

@abhijeetmanhas abhijeetmanhas requested a review from a team as a code owner June 21, 2020 19:09
nabobalis
nabobalis reviewed Jun 21, 2020
View reviewed changes
sunpy/net/base_client.py Outdated Show resolved Hide resolved
@nabobalis nabobalis requested a review from Cadair June 21, 2020 19:15
@nabobalis nabobalis added this to the 2.1 milestone Jun 21, 2020
@nabobalis nabobalis added the net Affects the net submodule label Jun 21, 2020
Cadair
Cadair reviewed Jun 22, 2020
View reviewed changes
Copy link
Member

@Cadair Cadair left a comment

Choose a reason for hiding this comment

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

This looks good, but it would be nice if you could test this without having to write more online tests. Can you just manually construct a QueryResponse object rather than calling out to search?

@pep8speaks
Copy link

pep8speaks commented Jun 22, 2020 •
edited

Hello @abhijeetmanhas! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-06-26 11:58:33 UTC

@abhijeetmanhas
Copy link
Contributor Author

This looks good, but it would be nice if you could test this without having to write more online tests. Can you just manually construct a QueryResponse object rather than calling out to search?

Done for all clients.

nabobalis
nabobalis reviewed Jun 22, 2020
View reviewed changes
sunpy/net/jsoc/jsoc.py Outdated Show resolved Hide resolved
Cadair
Cadair requested changes Jun 24, 2020
View reviewed changes
Copy link
Member

@Cadair Cadair left a comment

Choose a reason for hiding this comment

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

Other than the typo all looks good!

sunpy/net/dataretriever/sources/tests/test_eve.py Outdated Show resolved Hide resolved
@Cadair
Copy link
Member

Cadair commented Jun 26, 2020

Can you rebase to see if the CI will pass?

@nabobalis
Copy link
Contributor

test_calc_rad_loss_nokwags failed. Should be safe to ignore.

@nabobalis nabobalis merged commit 8c85186 into sunpy:master Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nabobalis nabobalis nabobalis approved these changes

@Cadair Cadair Cadair approved these changes

Assignees
No one assigned
Labels
net Affects the net submodule
Projects
None yet
Milestone
2.1
Development

Successfully merging this pull request may close these issues.

QueryResponse.show not flexible enough
4 participants
@abhijeetmanhas @pep8speaks @Cadair @nabobalis