Restructure astroquery#73
Conversation
|
I agree with all points laid out here. Perhaps merging those big PRs is not a bad idea; we'll just have to open new issues soon but if you're going to go ahead with the refactor today, I'd be inclined to let you do it against a cleanly merged master. |
Python 3 and docstring fixes. Merged as per #73 - we'll split off python3 and sphinx fixes into 2 separate PRs for future work
|
@keflavich @astrofrog Someone please review. |
There was a problem hiding this comment.
Any idea why this file is all green? I thought we had it already.
There was a problem hiding this comment.
But I moved it from astroquery/tests/test_fermi.py to astroquery/fermi/tests/test_fermi.py.
Apparently I changed the content in the same commit when I added the license line and now github shows this as a deletion and addition instead of a move.
Not nice, but I suggest to leave it like that ... I'll be more careful not to move and change content in the same commit in the future.
|
I didn't see any serious issues, but with so many small changes I'm not sure I would have caught any. Note that all the travis tests are failing, though, even some that were passing before, e.g. https://travis-ci.org/astropy/astroquery/jobs/6338243. I'm seeing gobs of errors with irsadust, which I suspect are caused by the refactor in some way. The only non-irsadust issue is this: |
There was a problem hiding this comment.
It might be best to find a different name for this module - maybe leave it as sim_votable.py since we use astropy.io.votable as votable - this may lead to confusion
|
I haven't figured out the causes for the test failures until now ... if you have a tip that would be great. |
|
I fixed the irsa_dust test errors. I forgot to rename The remaining simbad test error is not new, I believe. This test just wasn't executed before, because the test function name didn't start with My suggestion would be to merge this now and I'll open a new issue for that problem? |
|
The Python 3 tests on travis-ci also fail due to #80 . |
|
Yes, that sounds reasonable. Merge underway. |
I propose to restructure the astroquery files like this:
astroquery/service/service.pyis renamed toastroquery/service/core.pyastroquery/service/tests/, not all inastroquery/tests.astroquery/simbad/sim_parameters.pyis renamedastroquery/simbad/parameters.py.@keflavich @astrofrog What do you think?
I can do it now, but I don't know how rebasing will go for existing PRs. Do you want to merge e.g. #68 and #70 now and then we just keep working on them in new PRs after the restructure?