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

Raise proper exception when spectrum is not passed #228

Merged
merged 3 commits into from
Dec 19, 2019

Conversation

msrdinesh
Copy link
Contributor

@msrdinesh msrdinesh commented Dec 12, 2019

This pr is to address the issue 206

EDIT: Fixes #206

@msrdinesh msrdinesh changed the title Raise proper exception when vegamag is not passed Raise proper exception when Spectruem is not passed Dec 12, 2019
@msrdinesh msrdinesh changed the title Raise proper exception when Spectruem is not passed Raise proper exception when Spectrum is not passed Dec 12, 2019
@msrdinesh msrdinesh changed the title Raise proper exception when Spectrum is not passed Raise proper exception when spectrum is not passed Dec 12, 2019
Copy link
Contributor

@pllim pllim left a comment

Choose a reason for hiding this comment

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

There are some irrelevant diff in your commit. I think you need to rebase with the latest master upstream. Please let me know if you need help with this. Thanks!

@msrdinesh
Copy link
Contributor Author

Hii mam, I am new to git, please help me how to do it, or share any reference. Thanks!

@pllim
Copy link
Contributor

pllim commented Dec 13, 2019

You might want to back up your branch before you do this:

git fetch upstream master
git rebase upstream/master
git push -f origin master

@msrdinesh
Copy link
Contributor Author

Hii @pllim, I rebased my commit. Also, to close the 206, we need to add tests to check whether the exception is raised as you suggested in the comments. If possible please guide me through that also. Thanks!

@pllim
Copy link
Contributor

pllim commented Dec 17, 2019

@msrdinesh , try poke around test_observation.py and see if you can identify an obvious place to insert a test for this. Thanks! 🐱

@msrdinesh
Copy link
Contributor Author

Hii @pllim, I tried to add the test. There are a couple of errors. Please help me understand where I was wrong. Thanks!

@pllim
Copy link
Contributor

pllim commented Dec 19, 2019

@msrdinesh , I don't think the errors are related to your patch. Please rebase again in accordance to #228 (comment) and see what happens. Thanks!

@msrdinesh
Copy link
Contributor Author

Wow, it worked like charm! Thank you so much @pllim Please review the pr so, that we can close the issue 206.

Copy link
Contributor

@pllim pllim left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you so much for your patch and patience!

@pllim pllim merged commit 0167301 into spacetelescope:master Dec 19, 2019
@pllim pllim added this to the 0.3 milestone Dec 19, 2019
pllim added a commit that referenced this pull request Dec 19, 2019
@pllim pllim modified the milestones: 0.3, 0.2.1 Dec 20, 2019
pllim pushed a commit that referenced this pull request Dec 20, 2019
* Raise proper exception when spectrum is not passed

* Added test to check whether exception is raised properly
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.

Computing VEGAMAG fails when spectrum is not passed
2 participants