Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert activity type mapping to log-scale. #272

Merged
merged 3 commits into from
Dec 9, 2024

Conversation

beasleyjonm
Copy link
Contributor

The activity and potency types on Drug Central are log-scaled, so let's add the "p" in front of the activity types to reflect that.

The activity and potency types on Drug Central are log-scaled, so let's add the "p" in front of the activity types to reflect that.
@beasleyjonm beasleyjonm added the Biological Context QC Require validation of biological context to ensure accuracy and consistency label Nov 24, 2024
@EvanDietzMorris
Copy link
Contributor

Let's discuss this in a meeting perhaps, but these mappings correspond to the values in the drugcentral database, so we can't just change them or it will break them all. Also, the output of using these mappings are the predicates, not the values like ic50, so if we want this change to be represented in the graph we'll need to do something else.

It also looks like this only alters the comment in the drug central parser, and the actual mappings are in predicates.py

@beasleyjonm
Copy link
Contributor Author

Ah, my mistake. I believe my latest commit fixes the issue correctly. Let's discuss in an upcoming meeting.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eKathleenCarter check to confirm this. Can we make this biolink compliant?

@EvanDietzMorris
Copy link
Contributor

This code looks right now. As Kathleen comments, I think this would be a great example of a property that we would really like to be biolink compliant. Last time I looked I didn't see anything in the model that's an obvious fit, but maybe that's changed. Otherwise, we should reach out to the biolink folks and see if we can make a new property.

@EvanDietzMorris
Copy link
Contributor

I went ahead and switched the other parsers with these edge properties to using the constants we set up, so that they're all consistent. As well as adding this kind of property to biolink, we should probably audit the usage of them across the parsers to make sure they're used in the same way. (BINDING-DB, PHAROS, drugcentral, GtoPDB)

@EvanDietzMorris
Copy link
Contributor

Otherwise, I think we can go ahead and merge this in if you are both good with the state of it now.

Copy link
Contributor

@eKathleenCarter eKathleenCarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. we will follow up about making this biolink

@EvanDietzMorris EvanDietzMorris merged commit fcf6611 into master Dec 9, 2024
1 check passed
@EvanDietzMorris EvanDietzMorris deleted the drugcentral-logscale-potencies branch December 9, 2024 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Biological Context QC Require validation of biological context to ensure accuracy and consistency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants