BUG: mast.Observations.download_file local_path should accept directory

[bsipocz]

According to the docs local_path should be a directory, yet it expects a filename:

In [2]:    >>> from astroquery.mast import Observations
   ...:    ...
   ...:    >>> single_obs = Observations.query_criteria(obs_collection="IUE",obs_id="lwp13058")
   ...:    >>> data_products = Observations.get_product_list(single_obs)
   ...:    ...
   ...:    >>> product = data_products[0]["dataURI"]
   ...:    >>> print(product)
   ...:    >>> result = Observations.download_file(product)
mast:IUE/url/pub/iue/data/lwp/13000/lwp13058.elbll.gz
Downloading URL https://mast.stsci.edu/api/v0.1/Download/file?uri=mast:IUE/url/pub/iue/data/lwp/13000/lwp13058.elbll.gz to /Users/bsipocz/munka/devel/astroquery/lwp13058.elbll.gz ...
|==================================================================================| 185k/185k (100.00%)         0s

In [3]:    >>> from astroquery.mast import Observations
   ...:    ...
   ...:    >>> single_obs = Observations.query_criteria(obs_collection="IUE",obs_id="lwp13058")
   ...:    >>> data_products = Observations.get_product_list(single_obs)
   ...:    ...
   ...:    >>> product = data_products[0]["dataURI"]
   ...:    >>> print(product)
   ...:    >>> result = Observations.download_file(product,local_path='/tmp/')
mast:IUE/url/pub/iue/data/lwp/13000/lwp13058.elbll.gz
WARNING: Found cached file /tmp/ with size 3392 that is different from expected size 185727 [astroquery.query]
Downloading URL https://mast.stsci.edu/api/v0.1/Download/file?uri=mast:IUE/url/pub/iue/data/lwp/13000/lwp13058.elbll.gz to /tmp/ ...
|>---------------------------------------------------------------------------------|   0 /185k ( 0.00%)
---------------------------------------------------------------------------
IsADirectoryError                         Traceback (most recent call last)
<ipython-input-3-6916ea62cdaa> in <module>
      6 product = data_products[0]["dataURI"]
      7 print(product)
----> 8 result = Observations.download_file(product,local_path='/tmp/')

~/munka/devel/astroquery/astroquery/mast/observations.py in download_file(self, uri, local_path, base_url, cache, cloud_only)
    566                                             cache=cache, head_safe=True, continuation=False)
    567             else:
--> 568                 self._download_file(data_url, local_path,
    569                                     cache=cache, head_safe=True, continuation=False)
    570 

~/munka/devel/astroquery/astroquery/query.py in _download_file(self, url, local_filepath, timeout, auth, continuation, cache, method, head_safe, **kwargs)
    433         with ProgressBarOrSpinner(length, f'Downloading URL {url} to {local_filepath} ...',
    434                                   file=progress_stream) as pb:
--> 435             with open(local_filepath, open_mode) as f:
    436                 for block in response.iter_content(blocksize):
    437                     f.write(block)

IsADirectoryError: [Errno 21] Is a directory: '/tmp/'

In [4]:    >>> from astroquery.mast import Observations
   ...:    ...
   ...:    >>> single_obs = Observations.query_criteria(obs_collection="IUE",obs_id="lwp13058")
   ...:    >>> data_products = Observations.get_product_list(single_obs)
   ...:    ...
   ...:    >>> product = data_products[0]["dataURI"]
   ...:    >>> print(product)
   ...:    >>> result = Observations.download_file(product,local_path='/tmp/zizi.fits')
mast:IUE/url/pub/iue/data/lwp/13000/lwp13058.elbll.gz
Downloading URL https://mast.stsci.edu/api/v0.1/Download/file?uri=mast:IUE/url/pub/iue/data/lwp/13000/lwp13058.elbll.gz to /tmp/zizi.fits ...
|==================================================================================| 185k/185k (100.00%)         0s

cc @jaymedina and also cc @jdavies-st due to his recent involvement with cleaning up download-related issues in #2497

Apparenlty, local_path is not being used checked in the test suite nor the docs.

[pllim]

I got tripped by this. It is very unintuitive to ask for filename when the keyword says path.

p.s. jdavies-st is no longer at ST.

[jdavies-st]

Observations.download_file knows the dataURI it is fetching, and it should probably always use the same filename locally. So if someone passes either a path or a path plus filename to local_path, it should always just use the actual file's filename?

That would be a change in current behavior.

Or would we allow one to set local_path to a local directory or a directory and filename, and it would write to that filename? The latter is currently the only way it works.

p.s. jdavies-st is no longer at ST.

Which means I've had time recently to contribute to astroquery! 😅

[pllim]

Or would we allow one to set local_path to a local directory or a directory and filename, and it would write to that filename? The latter is currently the only way it works.

I think this is more intuitive.

[sean-lockwood]

This just tripped me up too.

[bsipocz]

PRs are welcome, also a name to tag from MAST (though all the issues are labeled).

[astrojimig]

@snbianco (new developer at MAST!) and I will work on this next week!