UKGovernmentBEIS / inspect_ai

Inspect: A framework for large language model evaluations
https://UKGovernmentBEIS.github.io/inspect_ai/
MIT License
400 stars 41 forks source link

minor code aesthetic improvements #15

Closed Lovkush-A closed 1 month ago

Lovkush-A commented 1 month ago

Would you be happy to receive PR's that contain minor improvements to the code, but which do not change functionality.

The two examples I have seen so far:

Example 1

            isinstance(self.value, str)
            or isinstance(self.value, int)
            or isinstance(self.value, float)
            or isinstance(self.value, bool)

could be

            isinstance(self.value, str, int, float, bool)

(Or, you could create alias Scalar = str | int | float | bool and use that throughout the file)

Example 2.

Function example_dataset can use Literal typehint, because you can list precisely what values you are expecting

def example_dataset(
    name: str,
    sample_fields: FieldSpec | RecordToSample | None = None,
) -> Dataset:
...
      name (str): Example dataset name. One of 'security_guide', 'theory_of_mind',
        'popularity', or 'biology_qa'
aisi-inspect commented 1 month ago

We aren't (yet) setup to take PRs on this repo (we actually do ongoing development on an internal repo and then sync to here every few days). So for now I would post these as issues and then we will resolve them internally and sync to here in due course.

daaain commented 1 month ago

I'd suggest picking up a tool like Ruff to deal with issues like this. That said, I'm not sure about example 2, but example 1 should be taken care of, see here and here.

aisi-inspect commented 1 month ago

@daaain Agreed re: Ruff, we actually are using it now (https://github.com/UKGovernmentBEIS/inspect_ai/blob/main/pyproject.toml#L14-L23) but it's not picking up those cases. Will add the rules you mentioned and cleanup any other cases of this.

aisi-inspect commented 1 month ago

Resovled with https://github.com/UKGovernmentBEIS/inspect_ai/commit/140c3d63ad42125213bffaa799147135c81857f3