fix: adding data specific p-value filters

[addramir]

✨ Context

Data specific lead p-value CS filters.

🛠 What does this PR implement

🙈 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]

Minor stylistic comment, and it is absolutely my preference, but I like to use as few variables as possible:

(
    # Reading Finngen finemapped dataset and convert it to study locus:
    FinnGenFinemapping.from_finngen_susie_finemapping(
        spark=session.spark,
        finngen_susie_finemapping_snp_files=finngen_susie_finemapping_snp_files,
        finngen_susie_finemapping_cs_summary_files=finngen_susie_finemapping_cs_summary_files,
    )
    # Flagging sub-significnat loci:
    .validate_lead_pvalue(
        pvalue_cutoff=FinngenFinemappingConfig().finngen_finemapping_lead_pvalue_threshold
    )
    # Write the output.
    .df.write.mode(session.write_mode).parquet(
        finngen_finemapping_out
    )
)

[addramir]

Please carefully check, I removed the variables but not sure

[DSuveges]

I might missing something, but could not found the flagging of the in-house finemapped datasets. Also you mentioned there would be specific p-value cutoff for UKBPPP.

[DSuveges]

There's one thing I'm not sure about. You have added finngen_finemapping_lead_pvalue_threshold to the relevant config, and refer to as pvalue_cutoff=FinngenFinemappingConfig().finngen_finemapping_lead_pvalue_threshold in the step. However, finngen_finemapping_lead_pvalue_threshold it not an argument for FinnGenFinemappingIngestionStep. Is it OK? Would all parameters in the config passed to the step? Would that cause any problem? @project-defiant , what do you think?

[DSuveges]

OK, double-checked with @project-defiant and @d0choa and all parameters in the setepConfig classes needs to be parameters in the init function of the step.

[DSuveges]

Same comment: EqtlCatalogueConfig().eqtl_lead_pvalue_threshold has to be passed as parameter of the step.

[DSuveges]

Same comment: FinngenFinemappingConfig().finngen_finemapping_lead_pvalue_threshold has to be an init parameter of the step.

[DSuveges]

Same comment.

[DSuveges]

Any parameter that are defined in the StepConfigs needs to be defined as parameters of the init methods in the respective classes otherwise the step would fail. The reason is that the step object would be initialised with an unexpected parameter.