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

Fix: Util function to ensure list of dicts all have same keys for hek #3240

Merged
merged 4 commits into from
Jun 25, 2019
Merged

Fix: Util function to ensure list of dicts all have same keys for hek #3240

merged 4 commits into from
Jun 25, 2019

Conversation

nabobalis
Copy link
Contributor

@nabobalis nabobalis commented Jun 22, 2019

Fixes #3238 via a large HACK.

Not really sure this is a sane way to do it however.

@nabobalis nabobalis added [BugFix] net Affects the net submodule util Issues relating to sunpy.util labels Jun 22, 2019
@nabobalis nabobalis added this to the 1.0.2 milestone Jun 22, 2019
@pep8speaks
Copy link

pep8speaks commented Jun 22, 2019

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

Line 172:101: E501 line too long (134 > 100 characters)

Comment last updated at 2019-06-25 19:13:16 UTC

@sunpy sunpy deleted a comment Jun 22, 2019
@sunpy sunpy deleted a comment Jun 22, 2019
@sunpy sunpy deleted a comment Jun 22, 2019
@sunpy sunpy deleted a comment Jun 22, 2019
@sunpy sunpy deleted a comment Jun 22, 2019
sunpy/util/util.py Outdated Show resolved Hide resolved
@Cadair
Copy link
Member

Cadair commented Jun 24, 2019

So, let me check I understand this, some records that are coming back from the HEK have different (in table parlance) columns than others? So you are making sure all the rows contain the superset of all columns?

@nabobalis
Copy link
Contributor Author

nabobalis commented Jun 24, 2019

Yes. if the column doesn't exist, we add it and set the value to None.

@Cadair
Copy link
Member

Cadair commented Jun 24, 2019

I think a different approach would be to use the columns= kwarg to Table, and just pass it a list of the superset of all columns. You could construct the set of all columns by doing set operations on the keys.

i.e.:

>>> d1 = {1:2, 3:4}; d2 = {3:4, 5:6}                                                                                                                                                                                                               
>>> s = set()                                                                                                                                                                                                                                      
>>> s.union(*map(lambda x: x.keys(), [d1, d2]))                                                                                                                                                                                                    
{1, 3, 5}

I can not promise this will work though, but it would mean not having to actually change the list of dicts.

@nabobalis
Copy link
Contributor Author

K

@Cadair
Copy link
Member

Cadair commented Jun 24, 2019

Also the constants thing needs a changelog entry of its own.

changelog/3240.bugfix.rst Outdated Show resolved Hide resolved
@nabobalis
Copy link
Contributor Author

Also the constants thing needs a changelog entry of its own.

Done.

@nabobalis
Copy link
Contributor Author

Trying to fix that astropy dev failure.

@nabobalis
Copy link
Contributor Author

What happens if you set the units and constants versions at the start of conftest.py

@nabobalis
Copy link
Contributor Author

nabobalis commented Jun 25, 2019

Database failures are back on Linux 3.7. Which is why it seems both the offline and astropy dev build failed.

@nabobalis
Copy link
Contributor Author

So I changed them to macos.

sunpy/sun/_constants.py Outdated Show resolved Hide resolved
@Cadair Cadair added the Merge When CI Passes Hit that merge button when it's all green! label Jun 25, 2019
@nabobalis nabobalis merged commit dbf39b4 into sunpy:master Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merge When CI Passes Hit that merge button when it's all green! net Affects the net submodule util Issues relating to sunpy.util
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hek client search method broken for SPoCA
3 participants