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

Added more descriptive error messages and hints for common usage mistakes #226

Merged
merged 30 commits into from Dec 8, 2022

Conversation

YoelShoshan
Copy link
Collaborator

When providing a a pipeline description, there are common pitfalls.

It's better to identify those common mistakes and provide clear instructions on how to fix it.

In this PR I added it for two cases:

Case 1:

You accidentally provide a class object instead of a class instance (very common mistake!)

Example exception message:

Exception: Error: expected an instance object, not a class object for <class 'fusedrug.data.protein.ops.aa_ops.OpToUpperCase'>
When creating a pipeline, such error can happen when you provide the following ops list description:
[SomeOp, {}]
instead of
[SomeOp(), {}]

Case 2:

you provided something that isn't a class or subclass of OpBase

Exception: Ops are expected to be instances of classes or subclasses of OpBase. The following op is not: 1234

In the future I think that we should also add:

  1. Validation of all of this at the pipeline constructor level, since it's already known then
  2. also validate the structure of the provided list of tuples with the pipeline op list

Yoel Shoshan and others added 30 commits April 14, 2022 17:13
… keys() items() and values() to our NDict, and tried to highlight more the faulting op in pipeline ops error
* Changed default deepdiff behavior to ignore nans in comparison,
* Added keys() items() and values() to our NDict (until now it returned empty iterables for those which is incorrect)
* Tried to highlight more the faulting op in pipeline ops error
…le_ids. fixed a typo in samplers.py and added a describe method to NDict
Copy link
Collaborator

@mosheraboh mosheraboh left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@YoelShoshan YoelShoshan merged commit 5717a12 into master Dec 8, 2022
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.

None yet

2 participants