search

astropy / astroquery

Functions and classes to access online data resources. Maintainers: @keflavich and @bsipocz and @ceb8
http://astroquery.readthedocs.org/en/latest/
BSD 3-Clause "New" or "Revised" License
706 stars 399 forks source link

Include warning in get_datalabs_path method for ehst when the data volume is not mounted in DataLabs #3059

Closed davidglt closed 4 months ago

davidglt commented 4 months ago

Include warning in get_datalabs_path method for ehst when the data volume is not mounted in DataLabs

pep8speaks commented 4 months ago

Hello @davidglt! 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! :beers:

Comment last updated at 2024-07-08 23:03:54 UTC
jespinosaar commented 4 months ago

Dear @bsipocz , let me explain the purpose of this method. Maybe you don't know, but we are developing ESA Datalabs, a science portal where users will be able to access their data using a Datalabs which, in the end, is a docker image, accesible via web interface, where users can mount the volumes associated to our data in read mode. This means, they don't have to download the data to use it. They can simply mount the volume and retrieve the appropriate file. Therefore, the purpose of this method is to add a very easy way, using our astroquery module, to retrieve the path of the file.

The point is that they should mount the volume before. If that is not the case, the file will not be accessible. So, we have modified the method in case this happens, so we can warn the user that he/she should mount the volume before using that path. It has no sense to use this method outside ESA Datalabs, as the file is not available in the user's local environment, but it is in Datalabs.

E.g. another application we have. The user can now download a Jupyter Notebook with a search and commands to download files from our UI and open it into Datalabs automatically. With this method, they can retrieve the files from the volume instead of downloading them.

jespinosaar commented 4 months ago

Please let us know if we should show more clearly the purpose of this method.

jespinosaar commented 4 months ago

This seems to be an unrelated issue. Please let us know if everything is ok, thanks @bsipocz !

bsipocz commented 4 months ago

I'm going ahead and rebase this to get rid of the merge commit and to hope the CI got just fixed, too.

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.

Project coverage is 67.54%. Comparing base (5d42c1f) to head (f1890ac). Report is 14 commits behind head on main.

Files Patch % Lines
astroquery/esa/hubble/core.py 0.00% 18 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3059 +/- ## ========================================== - Coverage 67.55% 67.54% -0.02% ========================================== Files 233 233 Lines 18289 18320 +31 ========================================== + Hits 12356 12375 +19 - Misses 5933 5945 +12 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bsipocz commented 4 months ago

Thanks @davidglt!

jespinosaar commented 4 months ago

OK, content wise this looks good. We have unrelated CI issues, and also the ESA servers seems to be down atm, but that shouldn't affect this anyway.

Yes, we had a hardware maintenance window yesterday, but ESA modules should be working today. Many thanks for your support @bsipocz !