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

chore: raise if several interfaces are found #113

Merged
merged 2 commits into from
Oct 3, 2024

Conversation

ThibaultFy
Copy link
Member

No description provided.

@ThibaultFy ThibaultFy marked this pull request as ready for review October 3, 2024 09:37
@ThibaultFy ThibaultFy requested a review from a team as a code owner October 3, 2024 09:37
@ThibaultFy ThibaultFy marked this pull request as draft October 3, 2024 09:37
@ThibaultFy ThibaultFy marked this pull request as ready for review October 3, 2024 11:53
Copy link
Contributor

@SdgJlbl SdgJlbl left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of that well-hidden source of bug 🕵️

@ThibaultFy
Copy link
Member Author

/e2e --tests camelyon,substrafl,sdk

@Owlfred
Copy link

Owlfred commented Oct 3, 2024

End to end tests: ❌ FAILURE

Jobs status:

“Rien ne sert de courir ; il faut partir à point.” ― Jean de La Fontaine (Le Lièvre et la Tortue)

Copy link
Contributor

@samlesu samlesu left a comment

Choose a reason for hiding this comment

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

LGTM

@ThibaultFy ThibaultFy merged commit 0c3f42d into main Oct 3, 2024
2 checks passed
@ThibaultFy ThibaultFy deleted the chore/raise-if-several-interfaces-are-found branch October 3, 2024 16:10
return found_interfaces[0]() # return interface instance
elif len(found_interfaces) > 1:
raise exceptions.InvalidInterfaceError(
f"Multiple interfaces found in module '{module_name}': {found_interfaces}"
Copy link

Choose a reason for hiding this comment

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

Change the error message no one knows what interface mean

Copy link

@jeandut jeandut Oct 7, 2024

Choose a reason for hiding this comment

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

"Multiple opener sub-classes defined in the opener.py file, this is not allowed."

Copy link
Member Author

Choose a reason for hiding this comment

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

The opener and the different subclasses will appear in module_name and found_interfaces

Copy link

Choose a reason for hiding this comment

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

I disagree you don't have the info of interface_class which is in this case opener

Copy link

Choose a reason for hiding this comment

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

Please add at least interface_class to the debug print statement

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.

5 participants