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.

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

10 display minimap window epic minimap 1 #47

Merged
merged 46 commits into from
Jul 16, 2024

Conversation

NicolasDelgadoL
Copy link
Collaborator

resolves #10

implemented a new 'minimap' feature that displays a smaller version of the image currently open in the main window. The minimap updates in real-time to reflect changes made to the image in the main window, such as adjustments in color and marker levels."

Changes Made

  • Added MinimapWindow class display the minimap
  • Added button in the menu bar to activate and deactivate the minimap
  • Tested with various image sizes and confirmed correct functionality.

All requirements from Issue #10 have been fulfilled (except the optional ones).

Changes were made in the gui.py, guiparts.py and menus.py files.

NicolasDelgadoL and others added 30 commits May 29, 2024 13:28
…minimap in gui, added button in the menu bar
… Window, twecked the update_image function, added documentation
…; appearance changes because of mode conversion
@NicolasDelgadoL NicolasDelgadoL marked this pull request as ready for review July 9, 2024 15:55
@sonjajk sonjajk self-requested a review July 9, 2024 16:22
@ShalinGhe ShalinGhe self-requested a review July 9, 2024 16:34
Copy link

@robertkrasniqi robertkrasniqi left a comment

Choose a reason for hiding this comment

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

Danke fürs anpassen, nun kann es getestet werden:)

  • Wenn ich Cellpose starte öffnet sich keine Minimap aber der Hacken ist gesetzt, also muss insgesamt zwei mal den Hacken unter File drücken um die Minimap zu öffnen. Danach funktioniert es aber ganz normal.

  • Außerdem ist mir ein kleiner Bug aufgefallen. Wenn das Minimap-Fenster im Vordergrund ist und ich mit dem Mauszeiger über die Slider hover werden diese Weiß anstatt wie normal blau zu bleiben. Ist das Minimap-Fenster nicht im Vordergrund passiert das nicht. Das sieht dann so aus:

image
  • Im weiteren tritt dieser Fehler auf wenn ich den Code ausführe:
image

@NicolasDelgadoL
Copy link
Collaborator Author

Hey Robert, jetzt sollten alle Bugs, die du erwähnt hast gefixed sein 👍

Copy link
Collaborator

@sonjajk sonjajk left a comment

Choose a reason for hiding this comment

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

Hi,
You said, that "The minimap updates in real-time to reflect changes made to the image in the main window, such as adjustments in color and marker levels", which isn't the case for me.
grafik
additionally, if I close the Minimap via the red x, the tick in the dropdown menue is still set, so that I have to remove it, bevore I can open the minimap again.
grafik

@NicolasDelgadoL
Copy link
Collaborator Author

NicolasDelgadoL commented Jul 11, 2024

Hello Sonja,

it would be helpful to have a bit more context as to figure out what could be going wrong; could you please tell us if this is the case for all images you test, or maybe just a specific format? We tested the program in its current version yesterday on both Mac and Windows and the updating of colors and markers seemed to be working just fine. Please make sure you have pulled the latest changes. Just for clarity, in the image you provided, the image is segmented: the masks are not meant to be reflected in the Minimap, if this is what you where referring to. Only the changes made to the markers and the color channels.

Regarding you other remark, the unchecking problem is also not present and could be due to working with an old version of the program as well.

robertkrasniqi
robertkrasniqi previously approved these changes Jul 11, 2024
Copy link

@robertkrasniqi robertkrasniqi left a comment

Choose a reason for hiding this comment

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

Die Dinge die ich genannt habe sind halb so schlimm, müssen also m.M.n. nicht unbedingt geändert werden.

Ich frage mich nur wieso so viele Zeilen geändert wurden wobei nichts am Code selbst geändert wurde? Das hat das Reviewen etwas schwierig gemacht und bin mir auch nicht sicher ob das so gemerged werden kann.

Dementsprechend würde ich jetzt Approven.
Good Job!:)

cellpose/gui/gui.py Show resolved Hide resolved
cellpose/gui/gui.py Show resolved Hide resolved
cellpose/gui/gui.py Outdated Show resolved Hide resolved
cellpose/gui/guiparts.py Outdated Show resolved Hide resolved
cellpose/gui/guiparts.py Show resolved Hide resolved
cellpose/gui/io.py Show resolved Hide resolved
@robertkrasniqi
Copy link

Hey Robert, jetzt sollten alle Bugs, die du erwähnt hast gefixed sein 👍

"Außerdem ist mir ein kleiner Bug aufgefallen. Wenn das Minimap-Fenster im Vordergrund ist und ich mit dem Mauszeiger über die Slider hover werden diese Weiß anstatt wie normal blau zu bleiben. Ist das Minimap-Fenster nicht im Vordergrund passiert das nicht. Das sieht dann so aus:", den Bug habe ich immer noch. Ist aber nicht schlimm da er die Nutzung der Software nicht beeinträchtigt.

ShalinGhe
ShalinGhe previously approved these changes Jul 11, 2024
Copy link
Collaborator

@ShalinGhe ShalinGhe left a comment

Choose a reason for hiding this comment

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

Habe die Minimap auf dem Mac getestet und es sieht wirklich gut aus.
Ich habe sowohl keinen Input, 2D files und layered pictures rein geladen und es funktioniert bei alles Inputs wie verlangt. Auch die Sliders passen ihre Veränderungen am Original Bild und Minimap an, der Code ist strukturiert und schön aufgeteilt und die Comments helfen gut ihn zu verstehen.
Das Fenster öffnet und schließt wie gewollt und setzt dementsprechend auch den Haken.
Als einziger Verbesserungsvorschlag ist wie gesagt nur die Magic Number mit der Größe.
Ansonsten gute Arbeit :)

cellpose/gui/guiparts.py Show resolved Hide resolved
@jakobjo jakobjo requested a review from sonjajk July 11, 2024 10:51
@robertkrasniqi robertkrasniqi self-assigned this Jul 11, 2024
@mariehartung mariehartung dismissed stale reviews from ShalinGhe and robertkrasniqi via 3efe02f July 14, 2024 08:00
Copy link
Collaborator

@sonjajk sonjajk left a comment

Choose a reason for hiding this comment

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

It was on windows, but now it works. Thank you. :)

Copy link
Collaborator

@sonjajk sonjajk left a comment

Choose a reason for hiding this comment

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

Now it works. Good job.

@RenzoRichter RenzoRichter merged commit bd3255d into main Jul 16, 2024
1 of 2 checks passed
@RenzoRichter RenzoRichter deleted the 10_Display_minimap_window_epic_minimap_1 branch July 16, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display minimap window epic minimap (1)
8 participants