Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add parameter validation #111

Merged
merged 9 commits into from
Sep 16, 2024
Merged

Conversation

Zethson
Copy link
Member

@Zethson Zethson commented Sep 16, 2024

Fixes #109

Before:

tissues = bt.Tissue.public(organism="Homo sapiens")
tissues

led to

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
File /vol/data/miniconda3/envs/similarity-lamin/lib/python3.10/site-packages/bionty/models.py:394, in BioRecord.public(cls, organism, source)
    393 try:
--> 394     return getattr(bionty_base, cls.__name__)(
    395         organism=organism, source=source_name, version=version
    396     )
    397 except (AttributeError, ValueError):

File /vol/data/miniconda3/envs/similarity-lamin/lib/python3.10/site-packages/bionty/base/entities/_tissue.py:26, in Tissue.__init__(self, organism, source, version, **kwargs)
     19 def __init__(
     20     self,
     21     organism: Optional[Literal["all"]] = None,
   (...)
     24     **kwargs,
     25 ) -> None:
---> 26     super().__init__(
     27         source=source,
     28         version=version,
     29         organism=organism,
     30         include_id_prefixes={"uberon": ["UBERON"]},
     31         include_rel="part_of",
     32         **kwargs,
     33     )

File /vol/data/miniconda3/envs/similarity-lamin/lib/python3.10/site-packages/bionty/base/_public_ontology.py:73, in PublicOntology.__init__(self, source, version, organism, include_id_prefixes, include_rel)
     72 # search in all available sources to get url and md5
---> 73 self._source_record = self._match_sources(
     74     self._all_sources,
     75     source=source,
     76     version=version,
     77     organism=organism,
     78 )
     80 self._organism = self._source_record["organism"]

File /vol/data/miniconda3/envs/similarity-lamin/lib/python3.10/site-packages/bionty/base/_public_ontology.py:230, in PublicOntology._match_sources(self, ref_sources, source, version, organism)
    229 if row.shape[0] == 0:
--> 230     raise ValueError(
    231         f"No source is available with {kwargs}\nCheck"
    232         " `.display_available_sources()`"
    233     )
    234 return row.to_dict(orient="records")[0]

ValueError: No source is available with {'organism': 'Homo sapiens'}
Check `.display_available_sources()`

During handling of the above exception, another exception occurred:

AttributeError                            Traceback (most recent call last)
Cell In[31], line 1
----> 1 tissues = bt.Tissue.public(organism="Homo sapiens")
      2 tissues

File /vol/data/miniconda3/envs/similarity-lamin/lib/python3.10/site-packages/bionty/models.py:409, in BioRecord.public(cls, organism, source)
    407             kwargs["organism"] = organism
    408     source = Source.filter(**kwargs).first()
--> 409 return StaticReference(source)

File /vol/data/miniconda3/envs/similarity-lamin/lib/python3.10/site-packages/bionty/models.py:35, in StaticReference.__init__(self, source_record)
     32 def __init__(self, source_record: Source) -> None:
     33     self._source_record = source_record
     34     super().__init__(
---> 35         source=source_record.name,
     36         version=source_record.version,
     37         organism=source_record.organism,
     38     )

AttributeError: 'NoneType' object has no attribute 'name'

After:

{
	"name": "ValueError",
	"message": "Invalid organism: Homo sapiens. Must be one of: all",
	"stack": "---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[3], line 1
----> 1 tissues = bt.Tissue.public(organism=\"Homo sapiens\")
      2 tissues

File ~/PycharmProjects/bionty/bionty/models.py:399, in BioRecord.public(cls, organism, source)
    395     return getattr(bionty_base, cls.__name__)(
    396         organism=organism, source=source_name, version=version
    397     )
    398 except InvalidParamError as e:
--> 399     raise ValueError(str(e)) from None
    400 except (AttributeError, ValueError):
    401     if source is None:

ValueError: Invalid organism: 'Homo sapiens'. Must be one of: 'all'"
}

Copy link

github-actions bot commented Sep 16, 2024

@github-actions github-actions bot temporarily deployed to pull request September 16, 2024 15:36 Inactive
Signed-off-by: zethson <[email protected]>
@github-actions github-actions bot temporarily deployed to pull request September 16, 2024 16:01 Inactive
Signed-off-by: zethson <[email protected]>
@github-actions github-actions bot temporarily deployed to pull request September 16, 2024 16:11 Inactive
Signed-off-by: zethson <[email protected]>
@github-actions github-actions bot temporarily deployed to pull request September 16, 2024 17:30 Inactive
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
@github-actions github-actions bot temporarily deployed to pull request September 16, 2024 17:41 Inactive
Signed-off-by: zethson <[email protected]>
@github-actions github-actions bot temporarily deployed to pull request September 16, 2024 17:46 Inactive
Signed-off-by: zethson <[email protected]>
@github-actions github-actions bot temporarily deployed to pull request September 16, 2024 17:50 Inactive
@Zethson Zethson marked this pull request as ready for review September 16, 2024 17:53
Signed-off-by: zethson <[email protected]>
@Zethson Zethson merged commit cde7cd3 into main Sep 16, 2024
2 checks passed
@Zethson Zethson deleted the feature/organism_non_existant_error branch September 16, 2024 17:57
@github-actions github-actions bot temporarily deployed to pull request September 16, 2024 17:58 Inactive
@falexwolf
Copy link
Member

This header is awesome!

image

But what is the after state? Is there an equally succinct way of phrasing it? :D

@@ -108,6 +108,26 @@ def __init__(
except AttributeError:
pass

def _validate_param(self, param_name: str, value: str | None) -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this equates type annotation checks, why not used typechecked or pydantic?

I assume it's because there is more going on.

Small note re naming: param in lamindb has a special meaning akin to feature. When we talk about plain Python function args, we say args aligned with Google-style docstrings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Should at some point update the "Paramters" header and replace it with "Arguments".

@falexwolf
Copy link
Member

Great PR!

Not so important: Why do we use Optional instead of | None here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better error messages for passed organisms that do not exist
2 participants