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

Modified /sunpy/instr/goes.py so that 'hek' is imported only in the f… #2938

Merged
merged 2 commits into from
Feb 23, 2019
Merged

Conversation

Akram9
Copy link
Contributor

@Akram9 Akram9 commented Feb 23, 2019

…unction that uses it.

Description

Solves the unexpected error as described in #2821 by moving import hek into get_goes_event_list.

Fixes #2821

@pep8speaks
Copy link

pep8speaks commented Feb 23, 2019

Hello @Akram9! Thanks for updating the PR.

Line 44:101: E501 line too long (130 > 100 characters)
Line 821:101: E501 line too long (103 > 100 characters)
Line 822:101: E501 line too long (103 > 100 characters)
Line 823:101: E501 line too long (103 > 100 characters)
Line 824:101: E501 line too long (103 > 100 characters)
Line 825:101: E501 line too long (103 > 100 characters)
Line 826:101: E501 line too long (103 > 100 characters)
Line 976:5: E122 continuation line missing indentation or outdented

Comment last updated on February 23, 2019 at 17:57 Hours UTC

@ghost
Copy link

ghost commented Feb 23, 2019

Thanks for the pull request @Akram9! Everything looks great!

@nabobalis nabobalis added this to the 1.0 milestone Feb 23, 2019
@nabobalis nabobalis added No Changelog Entry Needed Refactoring Code changes without affecting API (generally) labels Feb 23, 2019
@nabobalis
Copy link
Contributor

You have also pulled in changes to astropy_helpers. You need to remove them.

Since you have committed the change, the best way would be to run:

git reset HEAD~
git submodule update --init
git add .
git commit

That should commit the change to the file only (in theory).

sunpy/instr/goes.py Outdated Show resolved Hide resolved
@Akram9
Copy link
Contributor Author

Akram9 commented Feb 23, 2019

I am having some problem with astropy_helpers. I had missed your earlier comment on reverting the changes and now I am not able to push. What should I do?
Will deleting this PR and doing it again help?

PS: I don't get how the astropy changes occured.

@nabobalis
Copy link
Contributor

nabobalis commented Feb 23, 2019

No, deleting it won't really help. How it happend, I am unsure since in theory it should have been checked out at the correct version. I wonder if this is the bug mentioned in #2932.

You won't be able to normally push if you change the history, you'd have to force push.
You can do the same commands as above but with:

git reset HEAD~2
git submodule update --init
git status (check here that astropy_helpers is unchanged. 
git add .
git commit -m <message>
git push --force

You want to check first command, I forget if it is ~1 oor ~2.

@Akram9
Copy link
Contributor Author

Akram9 commented Feb 23, 2019

I did as per your above code and it seems better now. Please check.

@nabobalis
Copy link
Contributor

Yes that looks much beter now.

sunpy/instr/goes.py Outdated Show resolved Hide resolved
sunpy/instr/goes.py Outdated Show resolved Hide resolved
@Akram9
Copy link
Contributor Author

Akram9 commented Feb 23, 2019

So what do I do next?

@nabobalis
Copy link
Contributor

Nothing, this will be merged once the checks clear.

@Akram9
Copy link
Contributor Author

Akram9 commented Feb 23, 2019

Thanks a lot!

@nabobalis nabobalis merged commit 76b202f into sunpy:master Feb 23, 2019
@nabobalis
Copy link
Contributor

Thank you @Akram9.

@Akram9
Copy link
Contributor Author

Akram9 commented Feb 23, 2019

Oh, thanks to you people. This was my first PR, and an enjoyable one at that.

@Akram9 Akram9 deleted the feature branch February 24, 2019 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Changelog Entry Needed Refactoring Code changes without affecting API (generally)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected error on import of goes module
4 participants