Skip to content

Conversation

abretaud
Copy link
Contributor

@abretaud abretaud commented Oct 4, 2025

FOR CONTRIBUTOR:

  • I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • License permits unrestricted use (educational + commercial)
  • This PR adds a new tool or tool collection
  • This PR updates an existing tool or tool collection
  • This PR does something else (explain below)

Addressing comments from #7327

"I am wondering if the "Track Visibility" should not be "On when browser opens" by default." -> that's already the default

I'll finish adding gtf support next week

@abretaud abretaud mentioned this pull request Oct 4, 2025
@abretaud abretaud marked this pull request as ready for review October 8, 2025 13:08
@abretaud
Copy link
Contributor Author

abretaud commented Oct 8, 2025

Green and ready for review :)

GTF can be loaded know, although GFF is probably better for performances and test indexing

I also fixed a bug on synteny data that I hadn't seen in the previous PR

@lldelisle any other things to change?

Copy link
Contributor

@lldelisle lldelisle left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. I proposed a new change. Also I am surprised that you use simsize in your test. I would do a compare="diff" and lines_diff is the number of lines that contains the number of lines that contains a dataset code or something that can change from one instance to the other (but this is maybe too much work...).

@abretaud
Copy link
Contributor Author

Thanks! I've made the help/label changes, it's better indeed :)

For the tests, I used sim_size because there are many history/dataset ids and paths in both xml and json files, that keep on changing on each planemo run, I tried initially to use lines_diff, but it's painful to count the lines, and it will be painful too to update it. While using sim_size with delta=0, I'm quite sure that the json and xml will be exactly like expected, except the fixed-length-random-chunk-of-strings. There was also the idea of using asserts, but I think we'd miss a lot of small potential differences in the output. We'll see in the future if it's convenient or not like that

@bgruening
Copy link
Member

You can use the asserts in addition if you like. And for XML we have those fancy asserts: https://docs.galaxyproject.org/en/latest/dev/schema.html#attribute-is

might be also a good assert to have: https://docs.galaxyproject.org/en/latest/dev/schema.html#is-valid-xml

For json as well: https://docs.galaxyproject.org/en/latest/dev/schema.html#has-json-property-with-text

@mvdbeek
Copy link
Member

mvdbeek commented Oct 11, 2025

Could you also replace the .get_metadata() function call and use .metadata (a property) instead ?

@abretaud
Copy link
Contributor Author

Ok, I added the is-valid-xml test, and replaced the get_metadata() call. For detailed json/xml content asserts I'd vote for doing it later as I don't really have the time to do it now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants