LSSTDESC / tables_io

A small package to provide tools to read / write and convert tabular data for DESC
MIT License
1 stars 1 forks source link

Use `match` #94

Open sidneymau opened 3 months ago

sidneymau commented 3 months ago

There is a lot of code in tables_io that uses a series of if statements before falling back to an error. This should probably be a match statement now that those are supported in python.

For example, what is currently implemented as

tType = tableType(obj)
if tType == AP_TABLE:
    return obj
if tType == NUMPY_DICT:
    return apTable.Table(obj)
if tType == NUMPY_RECARRAY:
    return apTable.Table(obj)
if tType == PD_DATAFRAME:
    # try this: apTable.from_pandas(obj)
    return dataFrameToApTable(obj)
raise TypeError(f"Unsupported TableType {tType}")  # pragma: no cover

could be rewritten as

tType = tableType(obj)
match tType:
    case AP_TABLE:
        return obj
    case NUMPY_DICT:
        return apTable.Table(obj)
    case NUMPY_RECARRAY:
        return apTable.Table(obj)
    case PD_DATAFRAME:
        # try this: apTable.from_pandas(obj)
        return dataFrameToApTable(obj)
    case _:
        raise TypeError(f"Unsupported TableType {tType}")  # pragma: no cover

I think this would be a bit clearer and more robust to any possible conflicts with the type enum, etc.

sidneymau commented 3 months ago

Note: doing so would require dropping support for Python 3.9 (match was added in 3.10). I'm not sure what the overall opinion about python version support is, but worth keeping in mind.

sidneymau commented 3 months ago

I have no idea why the github bot thought this was appropriate for Review in the project tracker... this isn't even a PR