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

Esasky use ucd instead of json #3106

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

Conversation

emellega
Copy link
Contributor

No description provided.

@pep8speaks
Copy link

pep8speaks commented Sep 27, 2024

Hello @emellega! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-09-27 12:44:44 UTC

@emellega emellega force-pushed the esasky-use-UCD-instead-of-json branch from ba61cc3 to 2bbe42c Compare September 27, 2024 12:44
@bsipocz bsipocz added the esasky label Sep 27, 2024
@bsipocz bsipocz added this to the v0.4.8 milestone Sep 27, 2024
@bsipocz
Copy link
Member

bsipocz commented Sep 27, 2024

cc @imbasimba

@bsipocz
Copy link
Member

bsipocz commented Sep 30, 2024

Before diving into any reviews I would point out that there are two test failures that are likely related as I don't see them on main (though I haven't looked into the details).

They are "remote" tests, so CI doesn't pick them up, run them with pytest -P esasky -R

@emellega
Copy link
Contributor Author

emellega commented Oct 1, 2024

For me they are failing both on this branch and on main. Both failing tests are for the IUE mission, and they are the only tests for that mission. The first test that fails are failing because the request is timing out when trying to fetch the fits file http://sdc.cab.inta-csic.es/ines/jsp/SingleDownload.jsp?filename=LWR13178RL.FITS. I was sort of hoping that this was some temporary problem with the server hosting the IUE data, but since it has been failing for many days now, maybe it's worth investigating some more.

@imbasimba
Copy link
Contributor

I have contacted the IUE support team regarding the timeouts, but these issues seem unrelated to this particular PR.

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.

4 participants