Skip to content

Added tests for hitran package and download function#9

Merged
keflavich merged 4 commits into
keflavich:hitranfrom
migueldvb:hi
Nov 2, 2016
Merged

Added tests for hitran package and download function#9
keflavich merged 4 commits into
keflavich:hitranfrom
migueldvb:hi

Conversation

@migueldvb

@migueldvb migueldvb commented Nov 2, 2016

Copy link
Copy Markdown

I have included a new function to download data from the HITRAN database based on the hitran.py code that is released under the Free Academic License (permisive OSI-compatible license). There is also some spectroscopic data for the water molecule in the tests/data directory and a few simple tests for the HITRAN parser function.

@keflavich keflavich left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The test looks great. Some of the other features could use cleanup.

Comment thread astroquery/hitran/reader.py Outdated

ISO_INDEX = { 'id':0, 'iso_name':1, 'abundance':2, 'mass':3, 'mol_name':4 }

# Copied from hapi.py code

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

could you add a url for this code?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done

ISO_INDEX = { 'id':0, 'iso_name':1, 'abundance':2, 'mass':3, 'mol_name':4 }

# Copied from hapi.py code
ISO = {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

maybe it would be better to load this data from a JSON file? But it's OK as is for now.

Comment thread astroquery/hitran/reader.py Outdated
'numin=' + str(numin) + '&' + \
'numax=' + str(numax)
try:
req = urllib2.urlopen(url)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I prefer to use requests for these sorts of operations. If this works, again, it's fine, but is this both py2- and py3-compatible?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That is a good suggestion, will change it. This should also be python 2 and 3 compatible but have not tested it.

Comment thread astroquery/hitran/reader.py Outdated
chunk = req.read(CHUNK)
if not chunk: break
fp.write(chunk.decode('utf-8'))
log.info('{0} bytes written to {1}'.format(CHUNK, filename))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Probably better to use a progressbar rather than spit out this verbose information so often

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

When the query is large the server returns a chunked response so the progress bar does not work.

@migueldvb migueldvb left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Apparently it is not possible to serialize a dict with tuples as keys to json.
We can just leave it as it is now or transform the dictionary like here:
http://stackoverflow.com/a/7001667/605436

Comment thread astroquery/hitran/reader.py Outdated
'numin=' + str(numin) + '&' + \
'numax=' + str(numax)
try:
req = urllib2.urlopen(url)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That is a good suggestion, will change it. This should also be python 2 and 3 compatible but have not tested it.

Comment thread astroquery/hitran/reader.py Outdated
chunk = req.read(CHUNK)
if not chunk: break
fp.write(chunk.decode('utf-8'))
log.info('{0} bytes written to {1}'.format(CHUNK, filename))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

When the query is large the server returns a chunked response so the progress bar does not work.

@keflavich

Copy link
Copy Markdown
Owner

OK, it's fine to leave the dict-with-tuples-as-keys as is.

Regarding the progressbar - That's fine. It might be better to have a less verbose output (e.g., a single dot with no newline), but if typical files are only a few chunks, it's OK. If the server is nice, it will tell you the size of the file in the request headers (I think content-size), and then you can use a progressbar with that, just updating for each chunk retrieved, but if the server is not nice, your approach is the only way to let the user know something is happening.

@migueldvb

Copy link
Copy Markdown
Author

It looks like we only get the 'content-length' value for relatively small queries, so for large queries, when it would be more useful, there is no progress bar.

@keflavich keflavich merged commit de4aab6 into keflavich:hitran Nov 2, 2016
keflavich added a commit that referenced this pull request May 3, 2026
# This is the 1st commit message:

add HiGal module

more cleanup

add changelog

it is possible for a cutout to be missing; we need to allow for that.
Also, the hack I had before isn't needed when you appropriately query
for multiple wavelengths

remove some comments, still not done with the PR

minor correction

added basic docs

pep8 fixes.  The bracket closures DO match the indentation, though.
pep8 lies sometimes.

add local tests and cleanup some testing

more interactive tests revealed that my hacks didn't really work, this
commit has some fixes

move everything into astroquery.herschel.higal

update relative paths

whitespace

# This is the commit message #2:

update tests

# This is the commit message #3:

the higal server genuinely isn't showing anything when I search online now

# This is the commit message #4:

add HiGal module

# This is the commit message #5:

more cleanup

# This is the commit message #6:

add changelog

# This is the commit message #7:

it is possible for a cutout to be missing; we need to allow for that.
Also, the hack I had before isn't needed when you appropriately query
for multiple wavelengths

# This is the commit message #8:

remove some comments, still not done with the PR

# This is the commit message #9:

minor correction

# This is the commit message #10:

added basic docs

# This is the commit message #11:

pep8 fixes.  The bracket closures DO match the indentation, though.
pep8 lies sometimes.

# This is the commit message #12:

add local tests and cleanup some testing

# This is the commit message #13:

more interactive tests revealed that my hacks didn't really work, this
commit has some fixes
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.

2 participants