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

Specify "tier" for each extension #2123

Merged
merged 2 commits into from
Dec 2, 2024
Merged

Specify "tier" for each extension #2123

merged 2 commits into from
Dec 2, 2024

Conversation

lassoan
Copy link
Contributor

@lassoan lassoan commented Nov 25, 2024

By default, Tier 3 is assigned to all extensions - see list of exceptions (that assigned tier 1 or tier 5 instead) in the commit message.

Associated schema update: Slicer/Slicer#8066

Required for Slicer/Slicer#7474

@lassoan lassoan self-assigned this Nov 25, 2024
Copy link
Member

@pieper pieper left a comment

Choose a reason for hiding this comment

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

Thanks for moving ahead with this. It should help us clear the RP backlog.

There are several json schemas that end with ## and I'm not sure if that's going to hurt anything, otherwise I think we should go ahead and make any adjustments later.

DCMQI.json Outdated Show resolved Hide resolved
SlicerLookingGlass.json Outdated Show resolved Hide resolved
@lassoan
Copy link
Contributor Author

lassoan commented Nov 25, 2024

Thanks for the review. I've fixed the double # typos.

Let's hear fom @jcfr about the tier for SlicerLookingGlass. I assumed that Kitware is maintaining it, but maybe LookingGlass did not provide funding for the maintenance.

ModelClip.json Outdated Show resolved Hide resolved
SlicerRadiomics.json Outdated Show resolved Hide resolved
@fedorov
Copy link
Member

fedorov commented Nov 26, 2024

For the sake of completeness, can someone please link to the documentation that defines tiers and requirements?

@lassoan
Copy link
Contributor Author

lassoan commented Nov 26, 2024

For the sake of completeness, can someone please link to the documentation that defines tiers and requirements?

The requirements are in this pull request, too: https://github.com/Slicer/ExtensionsIndex/pull/2123/files#diff-18813c86948efc57e661623d7ba48ff94325c9b5421ec9177f724922dd553a35

It is mostly about splitting up the existing extension submission checklist, requiring less for tier 1 and requiring all for tier 5 extensions.

@lassoan lassoan force-pushed the add-tier branch 2 times, most recently from e14d5b8 to cc3a538 Compare November 26, 2024 20:39
@lassoan
Copy link
Contributor Author

lassoan commented Nov 26, 2024

Thanks for all the feedback. I've answered all the comments and checks are passing, so if I don't get any more comments then I'll merge this in about 2 hours.

Auto3dgm.json Outdated Show resolved Hide resolved
By default, Tier 3 is assigned to all extensions. See exceptions and rationale is below.

## Tier 5:

Extensions of high importance:

- DCMQI
- DebuggingTools
- DeveloperToolsForExtensions
- DICOMwebBrowser
- LanguagePacks
- MarkupsToModel
- MONAIAuto3DSeg
- NNUNet
- ParallelProcessing
- PyTorch
- QuantitativeReporting
- SegmentEditorExtraEffects
- SegmentMesher
- SequenceRegistration
- SlicerDcm2nii
- SlicerDMRI
- SlicerElastix
- SlicerFreeSurfer
- SlicerHeart
- SlicerIGSIO
- SlicerIGT
- SlicerMorph
- SlicerNeuro
- SlicerOpenAnatomy
- SlicerOpenIGTLink
- SlicerRT
- SlicerVirtualReality
- SlicerVMTK
- SurfaceWrapSolidify
- TotalSegmentator
- UKFTractography

## Tier 1:

### No maintainer/no response to issues in issue tracker:
- ABLTemporalBoneSegmentation
- AirwaySegmentation
- Auto3dgm
- BoneThicknessMapping
- Breast_DCEMRI_FTV (radiology-research/SlicerBreast_DCEMRI_FTV#7 without response)
- DRRGenerator
- CarreraSlice
- CleverSeg
- ModelClip (upstream did not respond to a PR with a minor update to fix an extension icon issue)
- NeedleFinder
- OrthodonticAnalysis
- NvidiaAIAssistedAnnotation
- RegularizedFastMarching
- ResectionPlanner
- Scoliosis
- SegmentationAidedRegistration
- SlicerITKUltrasound (failing to build across all platforms)
- SlicerToKiwiExporter
- TOMAAT
- ChangeTracker (Andrey Fedorov confirmed that he no longer provides support)

### Experimental:
- BigImage
- SlicerAIGT

### Functionality is no longer needed or useful for very few users:
- AnglePlanesExtension (tool in Q3DC now)
- DatabaseInteractor
- ErodeDilateLabel
- MeshToLabelMap
- CurveMaker (built-in markups feature now)
- SlicerConda

### Commercial license is needed for meaningful use:
- ConnectToSupervisely
- flywheel_connect
- FlywheelCaseIterator

### Too broad name that would mislead users
- IntensitySegmenter
- TissueSegmentation

### Build errors
- SlicerLookingGlass
- SlicerRadiomics
- SlicerDentalModelSeg (only works on linux)
Download from the current URL often fails with:

FailedFileLoadError: Failed to parse http://cdn.jsdelivr.net/gh/Slicer/Slicer@main/Schemas/slicer-extension-catalog-entry-schema-v1.0.1.json
@lassoan
Copy link
Contributor Author

lassoan commented Nov 28, 2024

@jamesobutler I think we should proceed merging this now, as is, and tweak it later.

We do not have any automated infrastructure for computing and enforcing recursive computation of tiers for dependencies. It would be hard to implement this for soft runtime dependencies. We could of course do the work manually but that would take away time from other, more impactful efforts. Since it is debatable if tiers should be applied recursively to dependent extensions at all, I would not introduce any rules for dependent extension tiers right now.

We could tweak the list of tier 5 extensions, but we don't need to do it now. Current tier 5 list is aspirational and we may remove some extensions from it as we are getting closer to cutting the new Slicer Stable Release (or we may adjust the list of tier 5 requirements).

We could delay the decision and discuss this further at developer meetings, but delaying this pull request would delay/complicate approval of several open pull requests and @jcfr's work on updating the extension catalog frontend.

@jamesobutler
Copy link
Contributor

jamesobutler commented Nov 28, 2024

Current tier 5 list is aspirational

Ok. Yeah as it relates to the user experience having the tiers represent the state of the extensions that they currently are in rather than what we would hope them to be would be the most beneficial (SlicerLookingGlass being Tier1 although aspires to be Tier5). Ultimately the user will have a poor experience if a Tier 5 is non-functional as they will lose trust in the rankings. Life happens and developers can’t always provide maintenance to an extension due to not having enough time or resources to fund the effort. There will definitely be at times where something we really want to have be available say SlicerDMRI and UKFTractography as Tier 5, but there isn’t the resources associated with a Tier 5 extension to reflect it to users. I think most extension developers would aspire to be Tier 5 even though realistically they are unable to provide that Tier 5 experience to the users consistently.

We could alternatively set all Tier 5 to be Tier 3 and then reasses the rankings more closely in the future to not hold up Tier related PRs. There would be Tier 1 and Tier 3 extensions specified currently.

@lassoan
Copy link
Contributor Author

lassoan commented Dec 2, 2024

We could alternatively set all Tier 5 to be Tier 3 and then reasses the rankings more closely in the future

We'll use the proposed list of Tier 5 extensions for the upcoming Slicer-5.8 release and any downgrading will happen as needed during the release process (if an extension has issues and we cannot fix it then it will be downgraded).

@lassoan lassoan merged commit 7b3823f into Slicer:main Dec 2, 2024
3 checks passed
@lassoan lassoan deleted the add-tier branch December 2, 2024 21:29
@jamesobutler
Copy link
Contributor

jamesobutler commented Dec 3, 2024

@lassoan It appears that SlicerNeuropacs was not given a tier ranking in this PR. SlicerNeuropacs extension was added to the index prior to this PR being integrated. It is also pointing to 1.0.0 of the schema file.

{
"$schema": "https://raw.githubusercontent.com/Slicer/Slicer/main/Schemas/slicer-extension-catalog-entry-schema-v1.0.0.json#",
"build_dependencies": [],
"build_subdirectory": ".",
"category": "Diffusion",
"scm_revision": "main",
"scm_url": "https://github.com/neuropacs/SlicerNeuropacs"
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants