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

1118 mass balance #1136

Merged
merged 9 commits into from
Nov 23, 2023
Merged

Conversation

pchelle
Copy link
Collaborator

@pchelle pchelle commented Nov 7, 2023

No description provided.

@pchelle pchelle marked this pull request as ready for review November 7, 2023 14:41
{
"Name": "Intracellular",
"Include": [
"Organism|**|Intracellular|Aciclovir*"
Copy link
Member

@Yuri05 Yuri05 Nov 20, 2023

Choose a reason for hiding this comment

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

I forgot what we decided: if the molecules list above includes only Aciclovir,
but I have e.g. Aciclovir-XY-Metabolite in my simulation:
will Aciclovir-XY-Metabolite be included via "Organism|**|Intracellular|Aciclovir*" or not?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I stated it in the vignette

Note that inclusion and exclusion criteria for molecule paths leverage wild card symbols (*).
...
Another use case is when a metabolite is defined using the molecule name as prefix.
Then, the metabolite can be included in the grouping.

{
  "Name": "Plasma for Aciclovir and metabolites", 
  "Include": ["Organism|**|Plasma|Aciclovir*"]
}

Copy link
Member

Choose a reason for hiding this comment

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

What I meant: the molecules list in the example contains Aciclovir only:

            "Molecules": [
                "Aciclovir"
            ],

Now if "Include": ["Organism|**|Plasma|Aciclovir*"] retrieves both Aciclovir and Aciclovir-XY-Metabolite: will the metabolite be included or not?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since only aciclovir was in molecules, no, they should not be retreived from the search.
(because the interesection between all the molecule paths from the compound in Molecules field and the Include field is used).

Copy link
Member

Choose a reason for hiding this comment

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

ok. it would be good to select another example for vignette, which incorporates metabolites as well - but let's keep Aciclovir for now AS IS.

Copy link
Member

Choose a reason for hiding this comment

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

created an issue for that: #1148

R/utilities-mass-balance.R Outdated Show resolved Hide resolved
R/utilities-mass-balance.R Outdated Show resolved Hide resolved
R/utilities-mass-balance.R Outdated Show resolved Hide resolved
R/utilities-mass-balance.R Outdated Show resolved Hide resolved
Copy link
Member

@Yuri05 Yuri05 left a comment

Choose a reason for hiding this comment

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

s. comments

@Yuri05

This comment was marked as resolved.

R/utilities-mass-balance.R Outdated Show resolved Hide resolved
R/utilities-mass-balance.R Outdated Show resolved Hide resolved
Comment on lines 35 to 42
if(!isEmpty(plotSettings$Name)){
sectionId <- defaultFileNames$resultID(length(massBalanceResults) + 1, "mass_balance")
massBalanceResults[[sectionId]] <- saveTaskResults(
id = sectionId,
textChunk = paste("###", plotSettings$Name, anchor(sectionId)),
includeTextChunk = TRUE
)
}
Copy link
Member

Choose a reason for hiding this comment

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

what happens if plotSettings$Name is empty?
Somehow I cannot see the else-part (where the default subchapter caption is written)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently, there is no sub section title added if plotSettings$Name is empty.

@Yuri05
Copy link
Member

Yuri05 commented Nov 22, 2023

1 test failed...

@Yuri05 Yuri05 merged commit 09c4076 into Open-Systems-Pharmacology:develop Nov 23, 2023
2 checks passed
@Yuri05
Copy link
Member

Yuri05 commented Nov 23, 2023

@pchelle
after the PR was merged 1 hour ago, I still don't see the mass balance article on the website (dev). Any idea why?
grafik

@pchelle pchelle deleted the 1118_mass_balance branch August 9, 2024 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants