Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Tool version sorting issue: Scanpy tools default to middle-tools #15580

Closed
nomadscientist opened this issue Feb 14, 2023 · 43 comments
Closed

Tool version sorting issue: Scanpy tools default to middle-tools #15580

nomadscientist opened this issue Feb 14, 2023 · 43 comments
Labels
area/tools area/UI-UX wontfix

Comments

@nomadscientist
Copy link
Contributor

For some reason, when you select different Scanpy tools , they default to random versions in the middle of the list.

This has burned us in a few training sessions now - can someone please figure out why they aren't defaulting to the most recent?

Screenshot 2023-02-14 at 15 59 37
Screenshot 2023-02-14 at 15 59 42

Screenshot 2023-02-14 at 16 00 23
@hexylena hexylena transferred this issue from galaxyproject/tools-iuc Feb 15, 2023
@hexylena hexylena changed the title Scanpy tools default to middle-tools Tool version sorting issue: Scanpy tools default to middle-tools Feb 15, 2023
@pcm32
Copy link
Member

pcm32 commented Feb 15, 2023

This is an issue with some unfortunate versioning issue with the formatting that one of us picked once (<version>+<num>+galaxy<num2>) which makes the standard python package that sorts versions dizzy. I have a PR almost ready to merge ebi-gene-expression-group/container-galaxy-sc-tertiary#266 that should push newer versions of the tools with out the problematic pattern, which should put a newer version at the top.

@hexylena
Copy link
Member

hexylena commented Feb 15, 2023 •
edited
Loading

I think there was some discussion as well (@nsoranzo? @bgruening?) to maybe have a smarter sort that knows how to sort +galaxy versions, rather than relying solely on the package tools version comparison (but I couldn't find it.)

@pcm32
Copy link
Member

pcm32 commented Feb 15, 2023

Yes, we discussed this problem in a previous issue, but cannot find it. I'm testing my changes on that PR above to merge, hopefully that will put back the newest tool at the top.

@mvdbeek mvdbeek closed this as completed Feb 15, 2023
@hexylena
Copy link
Member

@mvdbeek is there a related issue or PR that closes this out in 23.0? Did I miss something?

@mvdbeek
Copy link
Member

mvdbeek commented Feb 15, 2023 •
edited
Loading

We can't fix this on the Galaxy side, what @pcm32 is doing in ebi-gene-expression-group/container-galaxy-sc-tertiary#266 is the way to go.

@hexylena
Copy link
Member

We can't? Wasn't there some discussion that we could, by augmenting the comparison if and only if +galaxy was included, to parse the right half of that as a number?

@mvdbeek
Copy link
Member

mvdbeek commented Feb 15, 2023

We could do all sorts of fancy things, but there is an established convention, there is a fix that is easy enough (ebi-gene-expression-group/container-galaxy-sc-tertiary#266), so I don't think we need to descend into something that I don't think is as easy as what you're suggesting, however if someone wants to implement this and prove that it doesn't breaking sorting for any existing tool than that would obviously be fantastic and welcome. In the meantime I don't think we should track this as an open issue.

@hexylena
Copy link
Member

hexylena commented Feb 15, 2023 •
edited
Loading

I neither said, nor implied, "easy" in any of my comments. Only that I remembered some discussion and I was keen to re-discover said discussion in case it had additional context I was missing.

@mvdbeek
Copy link
Member

mvdbeek commented Feb 15, 2023

And I would like to close this issue as wontfix and set realistic expectations.

@hexylena
Copy link
Member

Yes, that's fine, I didn't reopen or suggest that we should. Again, just trying to find the conversation I remember.

@davelopez
Copy link
Contributor

Might be this one? #15071

@hexylena
Copy link
Member

Thank you @davelopez that looks like it!

@mvdbeek mvdbeek removed the kind/bug label Feb 15, 2023
@hexylena
Copy link
Member

hexylena commented Feb 15, 2023 •
edited
Loading

I mis-remembered it being focused on the general case of +galaxy, rather than this specific EBI tool case with the superfluous +, since I have tools that go up to +galaxy10 experiencing this a very similar issue.

Wholly agree that the 2 + case is better fixed other ways.

@bgruening
Copy link
Member

@hexylena I'm happy to "hide" the tools that are causing this to bring the linage back into shape.

@pcm32
Copy link
Member

pcm32 commented Feb 15, 2023 •
edited
Loading

Can we keep the conflicting tools visible (as they are the newest) until I push the newer version to the toolshed and that gets installed in eu? I wasn't aware that +galaxy10 would be conflictive as well, I was using that on my PR to fix this :-( . Should I I stick with +galaxy9? I expect the next update to be tool version related.

@hexylena
Copy link
Member

hexylena commented Feb 15, 2023 •
edited
Loading

@pcm32 it sorts as text, you can see it on the circos tools. It's a bit unfortunate since it presents the same way. Worst for me, with the circos tools, is that it loads an older version of the tool that had some more bugs.

And I'm stuck at that version since it hasn't been updated in a while.

image

You can try +galaxy91, perhaps that sorts about +galaxy9.

@nsoranzo
Copy link
Member

nsoranzo commented Feb 15, 2023 •
edited
Loading

I am planning to (find the time to) fix +galaxy10, but not the +3+galaxyX , there is already a partial fix PR from Nate #13570 .

@hexylena
Copy link
Member

fantastic @nsoranzo! Oh, and wow #13570 was also citing directly circos, brilliant.

@pcm32
Copy link
Member

pcm32 commented Feb 16, 2023

I have merged the tools with the version naming fix and they have been pushed to the toolshed, so now it should only be a matter for the galaxy.eu CI job that installs tools to get those updated @nomadscientist . Hopefully this should put those tools at the top.

@nsoranzo nsoranzo mentioned this issue Feb 16, 2023
Closed
@nomadscientist
Copy link
Contributor Author

@pcm32 so currently today, it's still not on top
Screenshot 2023-02-21 at 10 37 42
And I can also still confirm that Scanpy is defaulting to the middle tools when you click it
I'm not 100% following this, has this problem moved to a different issue link? Otherwise I'd say it's not closed?

@mvdbeek
Copy link
Member

mvdbeek commented Feb 21, 2023

This is the development repo for Galaxy, this is a tool issue. I can install the update on .org, only then will it appear there.

@mvdbeek
Copy link
Member

mvdbeek commented Feb 21, 2023

It's updated on usegalaxy.org, I think .eu runs tool updates in the weekend.
Screenshot 2023-02-21 at 12 13 37

@nomadscientist
Copy link
Contributor Author

WOOOOO! Awesome thanks!

@pcm32
Copy link
Member

pcm32 commented Feb 21, 2023 •
edited
Loading

I did push these tools last week (Wednesday or so probably), so they should be picked up by galaxy.eu, unless there was some issue on the CI this past weekend?

@nomadscientist
Copy link
Contributor Author

OK I just checked out .eu server today, and it still is automatically going for the older tools
Screenshot 2023-03-01 at 13 59 24

@nomadscientist
Copy link
Contributor Author

@mtekman do you happen to know why the tools are sticking to the old versions? Did you have to anchor that for a course at one point because newer tools were failing?

@mvdbeek
Copy link
Member

mvdbeek commented Mar 1, 2023 •
edited
Loading

That makes sense as .eu hasn't installed the new version

ok, i'm just blind ... this is a little odd

@mvdbeek
Copy link
Member

mvdbeek commented Mar 1, 2023 •
edited
Loading

I checked out the source on usegalaxy.eu:

Screenshot 2023-03-01 at 16 34 58

the side panel is explicitly pointing at the old version, I think a restart might be enough to fix that ?

ping @bgruening

@bgruening
Copy link
Member

The service is restarted every day, so I doubt a restart fixes it. I can hide older tools with strange version numbers if you like.

@pcm32
Copy link
Member

pcm32 commented Mar 1, 2023 •
edited
Loading

Could it be that the side panel has the tool version somehow hard coded in one of the files? I remember seeing this in the past somewhere.

@bgruening
Copy link
Member

@pcm32 we updated over the weekend, can you please recheck? If it still does not work, please let me know which versions I should hide.

@nomadscientist
Copy link
Contributor Author

IT WORKS! AHHHHH YAYYYY!

@bgruening
Copy link
Member

Awesome, one tab less in my browser. Thanks, everyone.

@mvdbeek mvdbeek mentioned this issue Apr 17, 2023
Closed
@nomadscientist
Copy link
Contributor Author

OK sorry team, but new related problem - so the tutorial links to the 1.8.1+galaxy0 tool on Github
Screenshot 2023-04-20 at 16 26 01

However, when you actually go to click the button, it takes you to the most recent tool (1.8.1+galaxy9). This is going to be a problem as the tutorial is only verified on the tools we test on it, so if it's not linking to the right tool version, this is going to burn us later. Any ideas why this is happening?

@nomadscientist
Copy link
Contributor Author

@bgruening @mvdbeek @pcm32 @hexylena
Hi folks, do you have any thoughts on this related issue? When you select the tool in Tutorial mode, it nevertheless defaults to the newest tool, meaning it's still over-riding the links in Tutorial mode (which aren't necessarily the newest, and certainly won't be over time).

@hexylena
Copy link
Member

hexylena commented May 2, 2023

if it's getting the wrong version in tutorial mode, then that's a different bug. we should see what's happening there. do you have a specific hands on box you can link me to?

@nomadscientist
Copy link
Contributor Author

Scanpy PlotEmbed does this; Scanpy RunPCA, etc. etc. I think it's all the Scanpy tools. You can see it a lot here: https://training.galaxyproject.org/training-material/topics/single-cell/tutorials/scrna-case_basic-pipeline/tutorial.html

@mvdbeek
Copy link
Member

mvdbeek commented May 2, 2023

Could you verify this against the latest versions ? I would assume that the majority of users are simply following along the instructions ... if you have more than one screen you're probably not using the overlay mode.

@hexylena
Copy link
Member

hexylena commented May 2, 2023

we should probably add a flag where tool versions are super key, and then expose the version more visibly for those.

@nomadscientist
Copy link
Contributor Author

Agreed - have you checked the buttons? Do you happen to know why the tutorial-mode isn't working to let us select the actual tutorial linked tools?

@hexylena
Copy link
Member

@nomadscientist I'm testing this morning, it appears to be functional on EU at least. Using your two examples there, runPCA and the subsequent tool in the "Filter..." single cell tutorial, both seem to load the requested tool version

@nomadscientist
Copy link
Contributor Author

nomadscientist commented Jun 30, 2023 via email

@nomadscientist
Copy link
Contributor Author

nomadscientist commented Jun 30, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/tools area/UI-UX wontfix
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@pcm32 @hexylena @bgruening @nsoranzo @mvdbeek @nomadscientist @davelopez