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

Add functions to interactive #315

Merged
merged 15 commits into from
Oct 9, 2024

Conversation

minhtien-trinh
Copy link
Contributor

As discussed with @LucaMarconato @melonora and @josenimo, I added two small functions to annotate the shapes/visualize the sample id. The two functions are:

  1. get_layer grabs the layer name and returns the layer if it matches the user input.
  2. add_text_to_polygons adds annotations/sample names visually to the chosen polygons/shapes layer

When the user visualizes their data, annotations or simple sample id's could help in identifying samples and aid in analysis.
Example usage would be:

sample_id_list = gdf.index.tolist()
polygon_layer_name = 'Control'
interactive.add_text_to_polygons(polygon_layer_name, text_annotations)
  1. Grab the sample id from the GeoDataFrame (index is used by default when linking a table with a shapes element)
  2. Define the shapes layer (name of the spatial element shapes)
  3. annotate (for now the default is: font size 10, red, center)

Ideally it will look like this:
image

minhtien-trinh and others added 2 commits September 9, 2024 14:42
In this commit two functions are added to the interactive class.
get_layer grabs the layer name and returns the layer if it matches the user input.
add_text_to_polygons adds annotations to the chosen polygons/shapes layer
Copy link

codecov bot commented Sep 9, 2024

Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.

Project coverage is 66.44%. Comparing base (69105e0) to head (f51edb7).
Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
src/napari_spatialdata/_interactive.py 84.61% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #315      +/-   ##
==========================================
+ Coverage   63.08%   66.44%   +3.35%     
==========================================
  Files          19       19              
  Lines        2636     2697      +61     
==========================================
+ Hits         1663     1792     +129     
+ Misses        973      905      -68     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@LucaMarconato
Copy link
Member

LucaMarconato commented Oct 3, 2024

Hi @minhtien-trinh thanks for your contribution! I reviewed the code and it looks great to me.

I ask you please two small adjustments before merging:

  • the codecov job doesn't pass, could you please add some tests to cover the new code added?
  • please update the changelog file. You can add the PR number #315 and your GitHub handle @minhtien-trinh, so this information will be parsed by the release notes.
  • I adjusted most the of pre-commits (in another PR), but some of them, related to this PR, are still failing. Please fix them so the CI status passes.

Thanks!

@minhtien-trinh
Copy link
Contributor Author

@LucaMarconato I wrote tests, modified the changelog and changed my code so that pre-commit succeeds. For some reason docs fails now and I'm uncertain why. Can you help me?

@Czaki
Copy link
Collaborator

Czaki commented Oct 7, 2024

It looks like bug in run on CI.
You may try to re-trigger CI pushing empty commit.

git commit -m "trigger ci" --allow-empty

@minhtien-trinh
Copy link
Contributor Author

@LucaMarconato Ready to merge :)

Copy link
Collaborator

@Czaki Czaki left a comment

Choose a reason for hiding this comment

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

Two remarks for performance and readability.

src/napari_spatialdata/_interactive.py Outdated Show resolved Hide resolved
src/napari_spatialdata/_interactive.py Outdated Show resolved Hide resolved
@LucaMarconato
Copy link
Member

Thanks for the contribution!

@LucaMarconato LucaMarconato merged commit a38e286 into scverse:main Oct 9, 2024
9 checks passed
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.

3 participants