-
Notifications
You must be signed in to change notification settings - Fork 16
[WIP] DO NOT MERGE refactor(RemoveAny): remove any types #773
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
base: main
Are you sure you want to change the base?
Conversation
Removing Any* types to make the code maintainable in the long run.
|
|
||
| def _is_coded_evaluator(self, evaluators: List[AnyEvaluator]) -> bool: | ||
| def _is_coded_evaluator( | ||
| self, evaluators: List[BaseEvaluator[Any, Any, Any]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this method will always categorize evaluators as coded. might break reporting
CC @Chibionos
| ] | ||
|
|
||
| AnyEvaluationItem = Union[EvaluationItem, LegacyEvaluationItem] | ||
| AnyEvaluator = Union[LegacyBaseEvaluator[Any], BaseEvaluator[Any, Any, Any]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Continuing the conversation #685 (comment) here - AnyEvaluator.
What is not clear to me is:
If a new class wants to be backward incompatible (intentionally) then extending NewClass from OldClass would be wrong, right ?
Are we saying that no matter what we would never have backward incompatibility ?
Thoughts @radu-mocanu @cristipufu @akshaylive @Chibionos ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are two concerns here:
- Backwards compatibility of the programmatic interfaces
- Backwards compatibility of the schema to represent evaluation sets
We don't care about ##1 right now because of no usage. However, ##2 is needed.
In the programmatic interfaces, we should avoid having to represent Any* = A | B because the entire internals of the library needs to know about both the versions. Instead, we need to transform the legacy schema to the new schema during loading.
We had a conversation regarding this and the conclusion was that the Legacy* will be removed in a few weeks.
Also updated the title of the PR to prevent merge.
Removing Any* types to make the code maintainable in the long run.