Introduce not given and apply it to dataset's target

[kevinmessiaen]

Description

Introduce not given and apply it to dataset's target

Related Issue

Type of Change

• 📚 Examples / docs / tutorials / dependencies update
• 🔧 Bug fix (non-breaking change which fixes an issue)
• 🥂 Improvement (non-breaking change which improves an existing feature)
• 🚀 New feature (non-breaking change which adds functionality)
• 💥 Breaking change (fix or feature that would cause existing functionality to change)
• 🔐 Security fix

Checklist

• I've read the CODE_OF_CONDUCT.md document.
• I've read the CONTRIBUTING.md guide.
• I've updated the code style using make codestyle.
• I've written tests for all new methods and classes that I created.
• I've written the docstring in Google format for all the methods and classes that I used.

[linear]

GSK-2118 Introduce NOT_GIVEN type

related to https://linear.app/giskard/issue/GSK-1956/support-target=false-to-suppress-warnings

[rabah-khalek]

Looks good to me @kevinmessiaen.

Could we also undo the changes done in https://github.com/Giskard-AI/giskard/pull/1531/files (the ones related to suppress the warning of target for LLM) and add the target=None in the BaseModel for when model_type == "text_generation" to suppress the warning?

[kevinmessiaen]

Looks good to me @kevinmessiaen.

Could we also undo the changes done in https://github.com/Giskard-AI/giskard/pull/1531/files (the ones related to suppress the warning of target for LLM) and add the target=None in the BaseModel for when model_type == "text_generation" to suppress the warning?

Not sure to understand the logic of the validation though, why to we validate the dataset target when validating model?

For me this validation is completely unrelated to the model validation and should be done when validating dataset only.
When uploading a model we should not confuse the user with warning that the dataset is missing a target, since it's unrelated.

[rabah-khalek]

that's exactly what I meant by my comment. We should remove it.
We had to do it to suppress the warning only for LLMs. We needed to do it inside validate_model just to know the model_type back then. But now people can put explicitly target=None to supress this warning

[kevinmessiaen]

that's exactly what I meant by my comment. We should remove it. We had to do it to suppress the warning only for LLMs. We needed to do it inside validate_model just to know the model_type back then. But now people can put explicitly target=None to supress this warning

Thanks for clarification, so it's what I did then, you can review the PR when you have time

[rabah-khalek]

Thanks @kevinmessiaen, looks good

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

96.3% Coverage
0.0% Duplication