cucapra / pollen

generating hardware accelerators for pangenomic graph queries
MIT License
24 stars 1 forks source link

Simplify sorted output of links #35

Closed sampsyo closed 1 year ago

sampsyo commented 1 year ago

Just a little thing: as mentioned in https://github.com/cucapra/pollen/pull/27/files#r1158837575, we can avoid defining our own ordering method and just use the one provided by dataclass.

The sort order is very slightly different now, since it's the order given by the fields in the dataclass definition: that's "from, from-orient, to, to-orient" as opposed to "from, to, from-orient, to-orient." I didn't test this exhaustively, but it matched on a couple of small inputs. It would be great to make sure this doesn't cause mismatches with odgi that I missed before we merge. If it does, we can reorder the fields in the Link definition to match, but we will also need to change other code that is currently relying on the order (namely, the code that constructs Link objects).

anshumanmohan commented 1 year ago

Oh super, thanks Adrian! I searched briefly for something like this before going with my silly handrolled version. I'll do a little due diligence and then merge.

anshumanmohan commented 1 year ago

I have confirmed that this does not create any problems!

sampsyo commented 1 year ago

Excellent!