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

Fixing LCO archive #911

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Fixing LCO archive #911

wants to merge 12 commits into from

Conversation

zemogle
Copy link
Contributor

@zemogle zemogle commented May 11, 2017

I'm opening this not because this is finished but so you can see the progress. I've not fixed the tests, which is next on my list but I do have working code which can auth with LCO and perform a simple query by object name and date constraints.

@bsipocz bsipocz added the lco label May 11, 2017
Copy link
Contributor

@keflavich keflavich left a comment

Choose a reason for hiding this comment

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

just some very minor comments

# Licensed under a 3-clause BSD style license - see LICENSE.rst
"""
Las Cumbres Observatory public archive Query Tool
===============
Copy link
Contributor

Choose a reason for hiding this comment

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

minor style: make the # of ='s same as # of characters above


"""
Las Cumbres Observatory
====
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

--------
While this section is optional you may put in some examples that
show how to use the method. The examples are written similar to
standard doctests in python.
Copy link
Contributor

Choose a reason for hiding this comment

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

just a reminder to remove this reminder text and replace it with something useful

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some useful text describing data format

Copy link
Member

@eteq eteq left a comment

Choose a reason for hiding this comment

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

@pep8speaks Suggest a diff

@bsipocz
Copy link
Member

bsipocz commented May 12, 2017

@eteq - Maybe the config is not properly set yet. Could we have a quick compare during coffeebreak to sunpy's. I'll also open an issue for them to expose the config to non org-wide admins similarly to the CI services.

Copy link

@OrkoHunter OrkoHunter left a comment

Choose a reason for hiding this comment

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

@pep8speaks suggest a diff

@astropy-bot
Copy link

astropy-bot bot commented Jun 12, 2018

Check results are now reported in the status checks at the bottom of this page.

@zemogle
Copy link
Contributor Author

zemogle commented Jun 12, 2018

Ok, I think I messed something up there. I'm currently working on this not trying to push a new PR. Sorry 😞

@bsipocz
Copy link
Member

bsipocz commented Jun 12, 2018

@zemogle: A rebase should solve this all.

assuming with git remote -v you have this:

upstream 	git@brigi.github.com:astropy/astroquery.git (fetch)
upstream 	git@brigi.github.com:astropy/astroquery.git (push)
origin	git@github.com:zemogle/astroquery.git (fetch)
origin	git@github.com:zemogle/astroquery.git (push)

Then git fetch --all and git rebase -i upstream/master. Latter brings up an editor, there please remove the last commit "Merge upstream". When done, git push origin lcogt -f

@bsipocz
Copy link
Member

bsipocz commented Jun 12, 2018

There was only a very minor change in the module when I've changed the tests from xfail to skip, so that may cause some conflict but they should be easy to resolve.

@zemogle
Copy link
Contributor Author

zemogle commented Jun 13, 2018

I'm having real issues getting the tests to run. I just get this error which looks unrelated to anything I've done:

_____________________________________________ ERROR collecting astroquery/lco/tests/test_lco_remote.py _____________________________________________
/Users/egomez/Applications/anaconda/envs/astroq/lib/python3.6/site-packages/_pytest/python.py:395: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
/Users/egomez/Applications/anaconda/envs/astroq/lib/python3.6/site-packages/py/_path/local.py:662: in pyimport
    __import__(modname)
lco/__init__.py:78: in <module>
    from .core import Lco, LcoClass
lco/core.py:43: in <module>
    import keyring
<frozen importlib._bootstrap>:961: in _find_and_load
    ???
<frozen importlib._bootstrap>:950: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:646: in _load_unlocked
    ???
<frozen importlib._bootstrap>:616: in _load_backward_compatible
    ???
/Users/egomez/Applications/anaconda/envs/astroq/lib/python3.6/site-packages/keyring-12.2.1-py3.6.egg/keyring/__init__.py:3: in <module>
    ???
<frozen importlib._bootstrap>:961: in _find_and_load
    ???
<frozen importlib._bootstrap>:950: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:646: in _load_unlocked
    ???
<frozen importlib._bootstrap>:616: in _load_backward_compatible
    ???
/Users/egomez/Applications/anaconda/envs/astroq/lib/python3.6/site-packages/keyring-12.2.1-py3.6.egg/keyring/core.py:13: in <module>
    ???
<frozen importlib._bootstrap>:961: in _find_and_load
    ???
<frozen importlib._bootstrap>:950: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:646: in _load_unlocked
    ???
<frozen importlib._bootstrap>:616: in _load_backward_compatible
    ???
/Users/egomez/Applications/anaconda/envs/astroq/lib/python3.6/site-packages/keyring-12.2.1-py3.6.egg/keyring/backend.py:10: in <module>
    ???
<frozen importlib._bootstrap>:961: in _find_and_load
    ???
<frozen importlib._bootstrap>:946: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:885: in _find_spec
    ???
<frozen importlib._bootstrap_external>:1157: in find_spec
    ???
<frozen importlib._bootstrap_external>:1131: in _get_spec
    ???
<frozen importlib._bootstrap_external>:1112: in _legacy_get_spec
    ???
<frozen importlib._bootstrap>:427: in spec_from_loader
    ???
<frozen importlib._bootstrap_external>:544: in spec_from_file_location
    ???
E     File "/Users/egomez/Applications/anaconda/envs/astroq/lib/python3.6/site-packages/entrypoints-0.2.3-py3.6.egg/entrypoints.py", line 98
E   SyntaxError: invalid escape sequence \s

@keflavich
Copy link
Contributor

Oh I've seen that before... but I think I saw it in beautifulsoup. It is some weird issue related to python3 rejecting "bad" string escape sequences. I don't remember how I solved it, unfortunately, but maybe try updating keyring?

@bsipocz
Copy link
Member

bsipocz commented Jun 13, 2018

I can't recall seeing this issue before.

@zemogle
Copy link
Contributor Author

zemogle commented Jun 13, 2018

The issue above was solved by down-grading keyring==11.0.0. The library is functional and I've added some tests.

@bsipocz bsipocz added this to the v0.3.9 milestone Jun 17, 2018
@astropy astropy deleted a comment from pep8speaks Jun 17, 2018
@bsipocz
Copy link
Member

bsipocz commented Jun 17, 2018

@zemogle - would you also add some narrative docs and examples?

@bsipocz bsipocz modified the milestones: v0.3.9, v0.3.10 Dec 6, 2018
@bsipocz bsipocz removed this from the v0.3.10 milestone Jun 3, 2019
@bsipocz bsipocz mentioned this pull request May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants