Claude generated feedback#1022
Conversation
| A name is required by all evaluators because it is used as the top-level key | ||
| in the results returned by a | ||
| :class:`citrine.informatics.workflows.PredictorEvaluationWorkflow`. | ||
| :class:`citrine.informatics.executions.predictor_evaluation.PredictorEvaluation`. |
There was a problem hiding this comment.
The actual issue in this file.
| class PredictorEvaluationWorkflowFactory(factory.DictFactory): | ||
| id = factory.Faker('uuid4') | ||
| name = factory.Faker("company") | ||
| description = factory.Faker("catch_phrase") | ||
| archived = False | ||
| evaluators = factory.List([factory.SubFactory(CrossValidationEvaluatorFactory)]) | ||
| type = "PredictorEvaluationWorkflow" | ||
| # TODO Create Trait and status_detail content | ||
| status = "SUCCEEDED" | ||
| status_description = "READY" | ||
| status_detail = [] | ||
|
|
||
|
|
There was a problem hiding this comment.
The actual issue in this file
|
|
||
|
|
||
|
|
||
| @pytest.fixture | ||
| def predictor_evaluation_workflow_dict(generic_entity, example_cv_evaluator_dict, example_holdout_evaluator_dict): | ||
| ret = deepcopy(generic_entity) | ||
| ret.update({ | ||
| "name": "Example PEW", | ||
| "description": "Example PEW for testing", | ||
| "evaluators": [example_cv_evaluator_dict, example_holdout_evaluator_dict] | ||
| }) | ||
| return ret |
There was a problem hiding this comment.
Actual issue in this file
| class CrossValidationEvaluator( | ||
| Serializable["CrossValidationEvaluator"], PredictorEvaluator | ||
| ): |
There was a problem hiding this comment.
It looks like Claude is breaking on 80 characters, not our setting of 99
anoto-moniz
left a comment
anoto-moniz
left a comment
There was a problem hiding this comment.
Most of its changes in factories and conftest are unhelpful nitpicks (e.g. swapping single quotes for double), some of which make the code less readable, in part by not following our line length of 99 characters.
But it did make some good catches (e.g. AutoMLModel should be AutoMLPredictor), so I'll grab the useful bits of this PR and repackage them.
|
I'm going to close this in favor of #1023, which pulls in the key feedback, and ignores the formatting adjustments. |
2 participants
Citrine Python PR
Results of Claude analysis.
I'm skipping issues it raised around
Sorry about the reformatting noise. Feel free to cherry-pick.
PR Type:
Adherence to team decisions