+
Skip to content

Allow only keeping a subset of slides for SlideSet #303

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 3 commits into from
Nov 15, 2021
Merged

Allow only keeping a subset of slides for SlideSet #303

merged 3 commits into from
Nov 15, 2021

Conversation

kheffah
Copy link
Collaborator

@kheffah kheffah commented Jul 26, 2021

Description

Allow only keeping a subset of slides for SlideSet. This would be useful when, for example, you want to exclude slides used in machine learning model training for cross validation.

Types of Changes

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

Issues Fixed or Closed by This PR

  • Fixes:

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 #303 (3c0da7d) into master (7cea37b) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #303   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines         1324      1328    +4     
  Branches       130       133    +3     
=========================================
+ Hits          1324      1328    +4
Impacted Files Coverage Δ
histolab/slide.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 7cea37b...3c0da7d. Read the comment docs.

@alessiamarcolini alessiamarcolini self-requested a review July 26, 2021 14:47
alessiamarcolini
alessiamarcolini requested changes Jul 26, 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.

Just a couple of stylistic changes :) the rest is good, thanks!!

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

kheffah commented Jul 26, 2021

@alessiamarcolini Thanks, 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.

Per comments

ernestoarbitrio
ernestoarbitrio approved these 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.

LGTM

alessiamarcolini
alessiamarcolini approved these changes Aug 2, 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.

LGTM!!

However we need to wait to merge this PR as it is scheduled to be in the 0.2.8 release :)

@alessiamarcolini
Copy link
Collaborator

@kheffah if you don't mind, I'll rebase on master and squash the "update" commits

@kheffah
Copy link
Collaborator Author

kheffah commented Aug 2, 2021

@kheffah if you don't mind, I'll rebase on master and squash the "update" commits

Sure thing, feel free :)

@alessiamarcolini alessiamarcolini force-pushed the slide-subset branch 2 times, most recently from 78817fc to b08fbdc Compare August 2, 2021 15:00
@kheffah kheffah mentioned this pull request Aug 7, 2021
Closed
@alessiamarcolini alessiamarcolini modified the milestones: 0.2.8, 0.4.0 Nov 8, 2021
kheffah and others added 3 commits November 12, 2021 17:22
@kheffah @alessiamarcolini
allow only keeping a subset of slides for SlideSet -- this would be u…
f091707 
…seful when, for example, you want to exclude slides used in machine learning model training for cross validation.
@kheffah @alessiamarcolini
Typing improvements and parameter style changes according to CR
f8886fc 
Co-authored-by: Alessia Marcolini <98marcolini@gmail.com>
@kheffah @alessiamarcolini
remove underscore from list comprehension in Slide class
3c0da7d
@alessiamarcolini alessiamarcolini merged commit 29ebb9a into master Nov 15, 2021
@alessiamarcolini alessiamarcolini deleted the slide-subset branch November 15, 2021 11:21
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 ready-for-merge
Projects
None yet
Milestone
0.4.0
Development

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