Skip to content
New issue

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

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

Already on GitHub? # to your account

BED parsing and rendering #38

Merged
merged 8 commits into from
Aug 2, 2017

Conversation

sourabh2k15
Copy link
Contributor

Please review and let me know if any changes are required.
preview : http://i.imgur.com/B0RhJTr.png

if (len > 7) {
feature.thickStart = fields[6];
feature.thickEnd = fields[7];
if(feature.thickEnd == feature.thickStart == feature.chromStart) feature.drawThickBlock = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

feature.chromStart has not been set - did you mean feature.start?
Also, feature.drawThickBlock is never used anywhere.

continue;
}
console.log(fields);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove console.logs


feature.subFeatures = subfeatures;
}
console.log(feature);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove console.logs

for(var j = 0; j < blockSizes.length; j++){
var subfeature = {};
subfeature.start = feature.start + parseInt(blockStarts[j]);
subfeature.end = subfeature.start + parseInt(blockSizes[j]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please give a radix argument (10) for parseInt

subfeatures.push(subfeature);
}

feature.subFeatures = subfeatures;
Copy link
Contributor

Choose a reason for hiding this comment

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

Only assign feature.subFeatures if subfeatures.length is not 0

view : Genoverse.Track.View.extend({
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't need to define a view here - you can leave the properties below on the track prototype (as the were previously). Also, populateMenu is a Controller function, not a View function, so it [probably] won't work at the moment

var blockSizes = fields[10].split(",");
var blockStarts = fields[11].split(",");
blockSizes.pop();
blockStarts.pop();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you pop these arrays?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because the blockSizes and blockStarts fields are of the form 1,2,3,4,
the ending comma leads to an extra empty element when we split the above string using ',' as delimiter.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, the example here https://genome.ucsc.edu/FAQ/FAQformat#format1 is

track name=pairedReads description="Clone Paired Reads" useScore=1
chr22 1000 5000 cloneA 960 + 1000 5000 0 2 567,488, 0,3512
chr22 2000 6000 cloneB 900 - 2000 6000 0 2 433,399, 0,3601

which has a trailing comma on blockSize but not on blockStart. I think it would be better to have them both defined as

fields[10 or 11].split(",").filter(function (n) { return n; })

That way we can be sure of not losing the last value if it exists

@sourabh2k15
Copy link
Contributor Author

I have implemented changes as per your comments. Also please help me figure out how to use thickStart and thickEnd. I did not get any hints from ucsc as to how they are rendered, so I haven't used them at the moment .

@simonbrent
Copy link
Contributor

re thickStart/thickEnd: what I understand from the description ("The starting position at which the feature is drawn thickly") is that between start and thickStart, and thickEnd and end, the subFeatures are drawn at one height, but between thickStart and thickEnd they are drawn with a greater height.

This reflects how transcripts are currently drawn, to a certain extent.

You can implement this by giving a thickHeight property to Track.File.BED, and then in Model.File.BED.parseData, set subfeature.height = this.prop('thickHeight') if subfeature.start and subfeature.end are between thickStart and thickEnd.

It may also be necessary to split a subfeature into two (one with height = thickHeight, the other without), if the subfeature is only partially within the thickStart-thickEnd region.

If there is a thickHeight region of the feature, the feature's height will need to be set to thickHeight too, in order to ensure correct positioning and drawing.

Incidentally, you're currently setting subfeature.height = 7 for each subfeature. It would be better to remove this line and set Track.File.BED.featureHeight = 7 instead of 6, if you think 6 is too small (7 and 10 are the heights used in Track.View.Transcript, so I think this is fine).

@sourabh2k15
Copy link
Contributor Author

Hello simon, I have added the change as you have said and removed pop() in favor of filter() to not lose last elements.

@simonbrent simonbrent merged commit e9bc74f into DECIPHER-genomics:gh-pages Aug 2, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants