ucl-exoplanets / TauREx3_public

Public release of TauREx3
https://taurex3-public.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
20 stars 11 forks source link

Bug: (CIA) loaders cannot handle pathlib Paths (feature request) #32

Open lwelzel opened 1 year ago

lwelzel commented 1 year ago

Bug Report/Feature Request: Support for pathlib Paths for loading (CIA) files

Description

Currently, when trying to load CIA files in the project, only str or List[str] types are accepted. However, it would be convenient to also be able to pass pathlib.Path objects to the data loaders.

Expected behavior

Pass strings or objects that can be represented as a string (e.g. Path instances) to the data loaders.

Actual behavior

An exception is raised (in CIACache.__getitem__()) when a Path instance is passed to the data loader via CIACache().set_cia_path().

Suspected cause

In taurex/cache/ciacache.py, an isinstance check is done that only accepts strings or a list of strings.

Suggested solution

To fix the issue, the isinstance check should be updated to also accept pathlib.Path instances.

File: taurex/cache/ciacache.py

import pathlib

# ...

def load_cia(self, cia_xsec=None, cia_path=None, pair_filter=None):
    """
    Main function to use when loading CIA files. Handles both
    cross sections and paths. Handles lists of either so lists of
    :class:`~taurex.cia.cia.CIA` objects or lists of paths can be used
    to load multiple files/objects

    Parameters
    ----------
    cia_xsec : :class:`~taurex.cia.cia.CIA` or :obj:`list` of :class:`~taurex.cia.cia.CIA` , optional
        Object(s) to include in cache

    cia_path : str, :class:`~pathlib.Path`, or :obj:`list` of str/:class:`~pathlib.Path`, optional
        Search path(s) to look for cias

    pair_filter : :obj:`list` of str , optional
        If provided, the cia will only be loaded
        if its pair name is in this list. Mostly used by the
        :func:`__getitem__` for filtering

    """

    from taurex.cia import CIA
    if cia_path is None:
        cia_path = self._cia_path

    self.log.debug('CIA XSEC, CIA_PATH %s %s', cia_xsec, cia_path)
    if cia_xsec is not None:
        if isinstance(cia_xsec, (list,)):
            self.log.debug('cia passed is list')
            for xsec in cia_xsec:
                self.add_cia(xsec, pair_filter=pair_filter)
        elif isinstance(cia_xsec, CIA):
            self.add_cia(cia_xsec, pair_filter=pair_filter)
        else:
            self.log.error('Unknown type %s passed into cia, should be a list, single \
                 cia or None if reading a path', type(xsec))
            raise Exception('Unknown type passed into cia')
    if cia_path is not None:
        if isinstance(cia_path, (str, pathlib.Path)):
            self.load_cia_from_path(cia_path, pair_filter=pair_filter)
        elif isinstance(cia_path, (list,)):
            for path in cia_path:
                self.load_cia_from_path(path, pair_filter=pair_filter)

# ...

The above code changes the isinstance check to also accept Path instances. This should allow Path instances to be passed to the data loaders without raising an exception.

TODO

Note: The code changes starts at roughly line 200 and requires the pathlib module. I did not check if this is also a problem elsewhere. Funnily enough, the exception that is raised reports the correct path (with the .db files) without issue. The data loading itself works fine with the change.

Thanks for the nice module!