+
Skip to content

Finer control of locate_tiles #304

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
Merged
merged 17 commits into from
Aug 12, 2021
Merged

Finer control of locate_tiles #304

merged 17 commits into from
Aug 12, 2021

Conversation

kheffah
Copy link
Collaborator

@kheffah kheffah commented Jul 26, 2021 •
edited
Loading

Finer control of locate_tiles including

  1. Allowing user to pass tiless to avoid re-extraction
  2. Passing individual colors to specific tiles (eg to emphasize specific regions of interest).

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Documentation

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@codecov
Copy link

codecov bot commented Jul 26, 2021 •
edited
Loading

Codecov Report

Merging #304 (34ec1a4) into master (ffd63f3) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #304   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines         1251      1264   +13     
  Branches       118       124    +6     
=========================================
+ Hits          1251      1264   +13
Impacted Files Coverage Δ
src/histolab/tiler.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffd63f3...34ec1a4. Read the comment docs.

@alessiamarcolini
Copy link
Collaborator

Hey @kheffah thanks for this PR!! First of all, could you split this commit into 3 separate commits?

@alessiamarcolini alessiamarcolini added the enhancement New feature or request label Jul 26, 2021
@kheffah
Copy link
Collaborator Author

kheffah commented Jul 26, 2021

@alessiamarcolini All done :)

ernestoarbitrio
ernestoarbitrio requested changes Jul 29, 2021
View reviewed changes
Copy link
Member

@ernestoarbitrio ernestoarbitrio left a comment

Choose a reason for hiding this comment

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

@kheffah thanks for this contribution. That's great. Just a couple of comments

ernestoarbitrio
ernestoarbitrio requested changes Jul 30, 2021
View reviewed changes
Copy link
Member

@ernestoarbitrio ernestoarbitrio left a comment

Choose a reason for hiding this comment

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

Per comments, please let me know if something is not clear enough

ernestoarbitrio
ernestoarbitrio requested changes Aug 2, 2021
View reviewed changes
Copy link
Member

@ernestoarbitrio ernestoarbitrio left a comment

Choose a reason for hiding this comment

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

I think we're close. Could you address my last comments please :D

ernestoarbitrio
ernestoarbitrio reviewed Aug 2, 2021
View reviewed changes
@ernestoarbitrio ernestoarbitrio added this to the 0.2.7 milestone Aug 9, 2021
@alessiamarcolini alessiamarcolini self-requested a review August 10, 2021 08:20
alessiamarcolini
alessiamarcolini requested changes Aug 10, 2021
View reviewed changes
Copy link
Collaborator

@alessiamarcolini alessiamarcolini left a comment

Choose a reason for hiding this comment

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

some stylistic changes + unit test code shouldn't have conditionals (see comments)

kheffah added 12 commits August 11, 2021 16:19
@kheffah @ernestoarbitrio
finer control of locate_tiles including 1. allowing user to pass tile…
8601a30 
…s to avoid re-extraction, 2. optionally not fading image, 3. passing individual colors to specific tiles (eg to emphasize specific regions of interest).
@kheffah @ernestoarbitrio
rever tiler.py for cleaner commit history
c4d3b26
@kheffah @ernestoarbitrio
allow user to pass tiles to tiler locate_tiles for efficiency
0ed0c79
@kheffah @ernestoarbitrio
allow user to control linewidth and outline color of individual tiles
4c031a9
@kheffah @ernestoarbitrio
allow user to opt out of fading clide image in locate_tiles
bc9f0ed
@kheffah @ernestoarbitrio
Revert "allow user to opt out of fading slide image in locate_tiles"
b6ebb1d 
This reverts commit a636668.
Instead we will default alpha to 255 to avoid unnecessary if statement.
@kheffah @ernestoarbitrio
change tiler unittest now that alpha default value is 128 again
b28a1a8
@kheffah @ernestoarbitrio
improve docstring and documentatino of tiler.
3032477
@kheffah @ernestoarbitrio
separately fix and validate tile outline and add a unit test for the …
3a63313 
…edge cases.
@kheffah @ernestoarbitrio
add extra edge case.
e7a1aa5
@kheffah @ernestoarbitrio
avoid making assumption about no of tiles when validating outline arg…
48c4e4c 
…ument of locate_tiles and use generators instead
@kheffah @ernestoarbitrio
add integration test cases for new outline parameter possibilities
9d23e96
kheffah and others added 3 commits August 11, 2021 16:22
@kheffah @ernestoarbitrio
move _tile_coords_and_outline_generator to right alphabetical position
d5c2412 
shorter line in DescribeRandomTiler

shorter line in DescribeRandomTiler
@kheffah @alessiamarcolini @ernestoarbitrio
Apply suggestions from code review
136f756 
Co-authored-by: Alessia Marcolini <98marcolini@gmail.com>
@ernestoarbitrio
blacken test tiler
b88f31e
alessiamarcolini
alessiamarcolini requested changes Aug 12, 2021
View reviewed changes
@alessiamarcolini alessiamarcolini merged commit 86db1a1 into master Aug 12, 2021
@ernestoarbitrio ernestoarbitrio deleted the flexible-locate-tiles branch August 12, 2021 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ernestoarbitrio ernestoarbitrio ernestoarbitrio approved these changes

@alessiamarcolini alessiamarcolini alessiamarcolini approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
0.3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@kheffah @alessiamarcolini @ernestoarbitrio
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载