Skip to content

Conversation

@keflavich
Copy link
Contributor

WIP: so far only a reader is implemented.

The HITRAN database (https://www.cfa.harvard.edu/hitran/) includes molecular line parameters and is more extensive than splatalogue, in particular including infrared transitions.

@migueldvb
Copy link
Member

This is working well. Any chance to have this PR merged?

@keflavich
Copy link
Contributor Author

Yeah, but it would be best to have tests. I don't even remember writing this code, but blame says I did...

@bsipocz
Copy link
Member

bsipocz commented Nov 2, 2016

Can't agree more with @keflavich, tests would be nice, and at least a very basic narrative docs page to show the usage of it.

@migueldvb
Copy link
Member

I will add some tests for the read_hitran_file function.

@keflavich
Copy link
Contributor Author

Once this passes, since we now have at least basic docs and tests, I'm happy to merge it. @migueldvb, let me know if there's more you'd like to add first.

@keflavich
Copy link
Contributor Author

Oh, we should add:

  • a changelog entry
  • an entry in index.rst


This module provides an interface to the `HITRAN`_ database API. It can
download a data file including transitions for a particular molecule in a given
wavenumber range. The file is downloaded in the `~hitran.cache_location`
Copy link
Member

Choose a reason for hiding this comment

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

You need to use the full path to make sphinx resolve it as an API link, e.g. ~astroquery.hitran.cache_location. Warnings for unresolvable links cause the build to fail.

========

This will download all transitions of the main isotopologue of water between
the wavenumbers of 3400 and 4100 cm\ :sup:`-1`\ .
Copy link
Member

Choose a reason for hiding this comment

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

Does this work for you locally? Singe backticks are special for sphinx, probably you will need to use double backticks here

Copy link
Member

Choose a reason for hiding this comment

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

This is working for me with sphinx 1.4.8

Miguel de Val-Borro and others added 2 commits November 5, 2016 22:02
This fixes a sphinx warning
Give cache directory path in HITRAN docs
@migueldvb
Copy link
Member

migueldvb commented Nov 6, 2016

There is a travis error that should not be related with these changes.
Error: Cannot remove astropy::aplpy-1.1.1-py35_0.tar.bz2 becaue it is pinned. Use --no-pin to override

@keflavich
Copy link
Contributor Author

keflavich commented Nov 6, 2016

This should probably affect other affiliated packages too. If we can get this fixed (or confirm that it's not something we should fix within astroquery), I think this is ready to merge.

EDIT: I restarted that build.

@bsipocz
Copy link
Member

bsipocz commented Nov 8, 2016

@keflavich - This is a weird issue (haven't seen it with other packages yet), and definitely has nothing to do with this PR.

@keflavich
Copy link
Contributor Author

@bsipocz OK, thanks for raising the aplpy issue. I think we'll mark this as 'xfail' and merge if we can't find a solution quickly, but with a note that we should unmark xfail once we figure out what's going on.

@bsipocz
Copy link
Member

bsipocz commented Nov 8, 2016

I probably have a workaround, but still waiting for travis to pass on it. I can hit the merge button on this if you prefer, or you can also add the failing build to the allow_failures: section in .travis.yml.

@bsipocz bsipocz merged commit aed3f93 into astropy:master Nov 8, 2016
@bsipocz
Copy link
Member

bsipocz commented Nov 8, 2016

Travis passed, so I'm merging this.

@bsipocz bsipocz modified the milestone: 0.3.4 Nov 21, 2016
@bsipocz
Copy link
Member

bsipocz commented Jan 3, 2017

@keflavich - Any particular reason for this module not following the common class API? Noticed while trying to get rid of the usage of commons.send_request() in favour of self._request...

@keflavich
Copy link
Contributor Author

@bsipocz no. probably just used the outdated template.

@keflavich keflavich deleted the hitran branch January 3, 2017 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants