feat: change StudyLocusId hashing method to md5 (and change StudyLocusId to string type)

[vivienho]

✨ Context

StudyLocusId is used as an identifier and does not need to be numerical. Changing it to string will make it easier on the backend side. Hashing strategy is changed to md5, which returns strings.

🛠 What does this PR implement

StudyLocusId is changed to string type in the schema and at relevant locations (mostly in tests).

The hashing strategy for generating the StudyLocusId is changed to md5.

A test (test_assign_study_locus_id__null_variant_id) was removed as validation steps elsewhere should have dropped null variant id cases before the assign_study_locus_id function.

🙈 Missing

🚦 Before submitting

• Do these changes cover one single feature (one change at a time)?
• Did you read the contributor guideline?
• Did you make sure to update the documentation with your changes?
• Did you make sure there is no commented out code in this PR?
• Did you follow conventional commits standards in PR title and commit messages?
• Did you make sure the branch is up-to-date with the dev branch?
• Did you write any new necessary tests?
• Did you make sure the changes pass local tests (make test)?
• Did you make sure the changes pass pre-commit rules (e.g poetry run pre-commit run --all-files)?

[DSuveges]

Given you were done so fast, I would recommend to update the logic to be more abstract allowing generalisation of the identifier generation (we will need this in other datasets, eg l2g). If you notice, you don't really need the arguments here:

    def assign_study_locus_id(
        study_id_col: Column,
        variant_id_col: Column,
        finemapping_col: Column = None,
    ) -> Column:

Because you have this list:

columns = [study_id_col, variant_id_col, finemapping_col]

This can be a simple array of column names, which I believe should be a class class attribute for StudyLocus dataset. So the class itself would define what columns needs to be hashed for the identifier and in which order. Also, I think the actual hashing logic:

 hashable_columns = [f.when(column.cast("string").isNull(), f.lit("None"))
                                 .otherwise(column.cast("string"))
                                 for column in columns]
        return f.md5(f.concat(*hashable_columns))

should be in the Dataset class. And the assign_study_locus_id method should wraps that function:

    @staticmethod
    def assign_study_locus_id( ) -> Column:
        self._generate_identifier(self.uniqueness_defining_columns).alias("studyLocusId")

Where:

• _generate_identifier is the hashing function in Dataset class, can be called from any dataset.
• uniqueness_defining_columns is a class attribute defined for the given dataset.
• This method returns the alias of the column, which is also dataset specific.

I have a tendency to over abstract everything, so it would be great to have a second opinion on this from @d0choa .

[DSuveges]

This is not a dealbreaker, and has no impact whatsoever: this column is not a "real" studyLocusId: this column is temporarily generated to identify original rows of the GWAS Catalog association dataset before explosion. But it is fine.

[DSuveges]

Lot of changes, all looks great, let's hope nothing breaks. 🤞🏻