Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Fixing v14 breaking changes #523

Merged
merged 11 commits into from
Feb 7, 2020

Conversation

jaclyn-taroni
Copy link
Member

Purpose/implementation Section

Here I am attempting to fix breaking changes that are coming up in CI as part of the v14 release.

@jaclyn-taroni jaclyn-taroni added the work in progress Used to label (non-draft) pull requests that are not yet ready for review label Feb 6, 2020
@cansavvy cansavvy marked this pull request as ready for review February 6, 2020 23:47
Copy link
Collaborator

@cansavvy cansavvy left a comment

Choose a reason for hiding this comment

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

I left comments on the changes I did so you know more about why I did them. Things look good from what I understand but I'm not really familiar with the fusion stuff at all.

@@ -20,20 +20,47 @@ jobs:
name: Sample Distribution Analyses
command: ./scripts/run_in_ci.sh bash "analyses/sample-distribution-analysis/run-sample-distribution.sh"

# TODO: The data files for CI need to be fixed https://github.com/AlexsLemonade/OpenPBTA-analysis/issues/527
Copy link
Collaborator

Choose a reason for hiding this comment

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

After the subset was redone, this doesn't rectify this?

Copy link
Member Author

Choose a reason for hiding this comment

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

No it did not - more details in #527

@@ -307,7 +304,7 @@ circos_map_plot(
```{r}
circos_map_transloc(transloc_df,
add_track = FALSE, # We change this to true to add on to our already existing plot
sample_names = samples_for_examples[1],
sample_names = sample(transloc_df$biospecimen_id1, 1),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I had to do this because not all samples have BNDs so this will kick back an error.

# Can't have identical y_min and y_max, this is just so CircleCI runs even if
# the subset data is wonky
if (y_min == y_max) {
y_max <- y_max + 0.001
Copy link
Collaborator

Choose a reason for hiding this comment

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

y_min and y_max are calculated based on the data, but with some of the subsets, you can get values that are identical and circlize doesn't like that. So this is my fix.

allFuseEmbry <- allFuseEmbry %>%
prepareOutput(specimensUnion)
if (!running_in_ci) {
allFuseEmbry <- allFuseEmbry %>%
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't really know what this means, but if it works 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Before, we skipped the step for generating the fusions of interest file for EPN samples because they were not represented in the subset. Now I'm switching and running the EPN, skipping the non-ATRT, non-MB embryonal fusions and genes of interest because those are not represented but the EPN samples are.

@jaclyn-taroni jaclyn-taroni added ci Related to continuous integration and removed work in progress Used to label (non-draft) pull requests that are not yet ready for review labels Feb 7, 2020
@jaclyn-taroni jaclyn-taroni merged commit 8a3df18 into AlexsLemonade:master Feb 7, 2020
@jaclyn-taroni jaclyn-taroni deleted the fix-v14-breaking branch February 7, 2020 00:22
@jaclyn-taroni jaclyn-taroni mentioned this pull request Feb 7, 2020
5 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ci Related to continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants