-
-
Notifications
You must be signed in to change notification settings - Fork 400
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
Heasarc: trying votable #2353
base: main
Are you sure you want to change the base?
Heasarc: trying votable #2353
Conversation
astroquery/heasarc/core.py
Outdated
@@ -252,7 +252,7 @@ def _parse_result(self, response, verbose=False): | |||
|
|||
try: | |||
data = BytesIO(response.content) | |||
return Table_read(data, hdu=1) | |||
return Table_read(data) |
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'll need to get rid of the Table_read
workaround, too.
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.
Ok, yes.
Though I think it will be still needed for the fallback, old interface has no VOTable.
I copied the code from #2333 to a test:
Currently it fails while trying to _convert_record2fits
. Maybe if it starts with fits it's more compatible. Some investigation is 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.
you'll need to get rid of the
Table_read
workaround, too.
I have been trying to work on this conversion to VOTable on my own. Is there any other reason why we need to keep the Table_read
function in other than silence the unit parsing?
For refernce:
def Table_read(*args, **kwargs):
if commons.ASTROPY_LT_5_1:
return Table.read(*args, **kwargs)
else:
return Table.read(*args, **kwargs, unit_parse_strict='silent')
I only ask since I in some testing with some of my own changes I've cut out this function and most tables work with VOTable perfectly fine without warnings.
I can submit a separate PR with more info if that would be more appropriate.
Hello @volodymyrss! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
|
in response to #2333 and #2349, this is an attempt to use VOTable backend of heasarc to resolve problems with unit conversion.
It needs to be determined if this is a viable approach.