{"action":"created","issue":{"url":"https://api.github.com/repos/rust-bio/rust-bio/issues/47","repository_url":"https://api.github.com/repos/rust-bio/rust-bio","labels_url":"https://api.github.com/repos/rust-bio/rust-bio/issues/47/labels{/name}","comments_url":"https://api.github.com/repos/rust-bio/rust-bio/issues/47/comments","events_url":"https://api.github.com/repos/rust-bio/rust-bio/issues/47/events","html_url":"https://github.com/rust-bio/rust-bio/pull/47","id":142749971,"number":47,"title":"FMIndex trait & suffix-array sampled version","user":{"login":"cramertj","id":5963049,"avatar_url":"https://avatars.githubusercontent.com/u/5963049?v=3","gravatar_id":"","url":"https://api.github.com/users/cramertj","html_url":"https://github.com/cramertj","followers_url":"https://api.github.com/users/cramertj/followers","following_url":"https://api.github.com/users/cramertj/following{/other_user}","gists_url":"https://api.github.com/users/cramertj/gists{/gist_id}","starred_url":"https://api.github.com/users/cramertj/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cramertj/subscriptions","organizations_url":"https://api.github.com/users/cramertj/orgs","repos_url":"https://api.github.com/users/cramertj/repos","events_url":"https://api.github.com/users/cramertj/events{/privacy}","received_events_url":"https://api.github.com/users/cramertj/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"johanneskoester","id":1858646,"avatar_url":"https://avatars.githubusercontent.com/u/1858646?v=3","gravatar_id":"","url":"https://api.github.com/users/johanneskoester","html_url":"https://github.com/johanneskoester","followers_url":"https://api.github.com/users/johanneskoester/followers","following_url":"https://api.github.com/users/johanneskoester/following{/other_user}","gists_url":"https://api.github.com/users/johanneskoester/gists{/gist_id}","starred_url":"https://api.github.com/users/johanneskoester/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/johanneskoester/subscriptions","organizations_url":"https://api.github.com/users/johanneskoester/orgs","repos_url":"https://api.github.com/users/johanneskoester/repos","events_url":"https://api.github.com/users/johanneskoester/events{/privacy}","received_events_url":"https://api.github.com/users/johanneskoester/received_events","type":"User","site_admin":false},"milestone":null,"comments":3,"created_at":"2016-03-22T19:13:15Z","updated_at":"2016-03-22T20:06:30Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/rust-bio/rust-bio/pulls/47","html_url":"https://github.com/rust-bio/rust-bio/pull/47","diff_url":"https://github.com/rust-bio/rust-bio/pull/47.diff","patch_url":"https://github.com/rust-bio/rust-bio/pull/47.patch"},"body":"I've been trying to rework FMIndex in accordance with the discussions in #38 (making FMIndex a trait) and #45 (allowing suffix array sampling). Here's my first pass-- I've tried to preserve most of the original design, while allowing different suffix array ownership/sampling patterns.\r\n\r\nI wasn't quite sure how best to implement FMDIndex to allow for all these different ownership patterns (I could make an FMDIndex for each, but that would be silly), so I allowed an FMIndex to be coerced into an FMDIndex using `as_fmdindex`.\r\n\r\nAdditionally, something isn't quite right with the suffix array bit. I think it might be related to the issue with #46. `test_smems_sampled` isn't passing, but closer inspection of the log reveals it's doing precisely the right thing (moving to the location of next left character in the suffix array while incrementing the offset) up until it hits a sentinel, at which point the ordering seems to be messed up. My guess is that the sentinels are somehow out of order, but I couldn't track down an obvious fix."},"comment":{"url":"https://api.github.com/repos/rust-bio/rust-bio/issues/comments/199996728","html_url":"https://github.com/rust-bio/rust-bio/pull/47#issuecomment-199996728","issue_url":"https://api.github.com/repos/rust-bio/rust-bio/issues/47","id":199996728,"user":{"login":"cramertj","id":5963049,"avatar_url":"https://avatars.githubusercontent.com/u/5963049?v=3","gravatar_id":"","url":"https://api.github.com/users/cramertj","html_url":"https://github.com/cramertj","followers_url":"https://api.github.com/users/cramertj/followers","following_url":"https://api.github.com/users/cramertj/following{/other_user}","gists_url":"https://api.github.com/users/cramertj/gists{/gist_id}","starred_url":"https://api.github.com/users/cramertj/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cramertj/subscriptions","organizations_url":"https://api.github.com/users/cramertj/orgs","repos_url":"https://api.github.com/users/cramertj/repos","events_url":"https://api.github.com/users/cramertj/events{/privacy}","received_events_url":"https://api.github.com/users/cramertj/received_events","type":"User","site_admin":false},"created_at":"2016-03-22T20:06:30Z","updated_at":"2016-03-22T20:06:30Z","body":"That would actually be my preference, and I considered structuring the implementation that way. The issue I ran into is that getting a value from the suffix array then requires both the BWT and the Less array, so is essentially still strongly linked to the FMIndex, as the FMIndex currently takes full ownership of both. It could perhaps accept them as parameters, but then implementing a common interface for the two suffix arrays would be impossible (unless, that is, we add a method to the regular suffix array that takes those parameters and simply ignores them).\r\n\r\nAdditionally, I wasn't sure what other tools in the library used suffix arrays (if any). Do I need to be on the lookout for making breaking changes?"}}