search

ontodev / robot

ROBOT is an OBO Tool
http://robot.obolibrary.org
BSD 3-Clause "New" or "Revised" License
264 stars 74 forks source link

diff feature request: sort axioms in Added and Removed sections in Markdown output #1192

Open jclerman opened 7 months ago

jclerman commented 7 months ago

Currently the Markdown output of robot diff ... -f markdown seems to randomly order the lines under "Added" or "Removed" for each changed entity (e.g., class).

I think the output would be much easier to read (and compare, when needed) if it had a canonical sort-order for those lines. I'd prefer an order like:

jamesaoverton commented 7 months ago

I believe that https://github.com/balhoff/owl-diff is doing the work for ROBOT here. Do you have suggestions @balhoff ?

balhoff commented 7 months ago

I agree that a standard ordering would make sense. I'm not sure how quickly I can get to that—PRs are definitely welcome.

jclerman commented 5 months ago

I was looking at this over the weekend. If I understand the code correctly, the lines that govern the sort-order within each altered OWL entity (for MarkDown output, anyway) are https://github.com/balhoff/owl-diff/blob/master/src/main/scala/org/geneontology/owl/differ/render/MarkdownGroupedDiffRenderer.scala#L41-L43. I'm not sure if I'm getting that right though, since in my hands, the sort seems to be random, whereas that code suggests that some ordering is expected.

@balhoff, should I open an issue against https://github.com/balhoff/owl-diff to track this request?

balhoff commented 5 months ago

@balhoff, should I open an issue against https://github.com/balhoff/owl-diff to track this request?

@jclerman that sounds good. Looking at it, I think the lines are "mostly sorted". It looks like annotations should be in alphabetical order by their toString representation, and overall axioms are in alphabetical order by their axiom type (but things like subclass axioms may not be reliably sorted against each other). I think your ordering makes sense except that I would prefer the annotation axioms to come after the declaration axiom.

balhoff commented 22 hours ago

@jclerman sorry for the slow response—I've got a PR at #1227 which hopefully addresses your issue.

jclerman commented 13 hours ago

Thank you! That sounds great. I'll try it out soon - tied up with some other things myself at the moment.