pepkit / pepdbagent

Database for storing sample metadata
BSD 2-Clause "Simplified" License
2 stars 1 forks source link

Change calling signature for `get_project` #33

Closed nleroy917 closed 1 year ago

nleroy917 commented 1 year ago

I'm wondering if we can change the calling signature of Connection.get_project. Currently, its this:

    def get_project(
        self,
        namespace: str = DEFAULT_NAMESPACE,
        name: str = None,
        tag: str = DEFAULT_TAG,
    ) -> Union[peppy.Project, None]:

My suggestion is to set namespace and tag to default to None. Then when inside the function, run checking functions and assign accordingly:

if namespace is None:
   namespace = DEFAULT_NAMESPACE
if tag is None:
   tag = DEFAULT_TAG

I think this is better because users of get_project (Me) don't need to care about the values of DEFAULT_NAMESPACE or DEFAULT_TAG.

async def get_a_pep(
    db: Connection = Depends(get_db),
    namespace: str = example_namespace,
    pep_id: str = example_pep_id,
    tag: str = None
):
    """
    Fetch a PEP from a certain namespace
    """
    if tag is not None:
        proj = db.get_project(namespace, pep_id, tag=tag)
    else:
        proj = db.get_project(namespace, pep_id)

Now becomes:

async def get_a_pep(
    db: Connection = Depends(get_db),
    namespace: str = example_namespace,
    pep_id: str = example_pep_id,
    tag: str = None
):
    """
    Fetch a PEP from a certain namespace
    """
    proj = db.get_project(namespace=namespace, name=pep_id, tag=tag)

Which is a bit cleaner and I don't have to check for the value of tag. It's a small thing, but could make usage of the function cleaner. Not sure if this is good practice or not. Others can weigh in.