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

Do not require every plugin recorded in the model bundle to be installed when using it #1916

Merged
merged 1 commit into from Sep 8, 2023

Conversation

AdeelH
Copy link
Collaborator

@AdeelH AdeelH commented Sep 8, 2023

Overview

The model bundles record every rastervision plugin that was installed when they were created. Previously, it was required to have all those plugins installed when using those bundles even if not actually needed or otherwise an error would be raised. This has been especially relevant ever since rastervision_gdal_vsi was made optional and not included in the default pip install--meaning users would not have been able to use bundles from the model zoo without installing rastervision_gdal_vsi first.

This PR removes this requirement. Now, the missing plugins are just logged as a warning and no error is raised.

This PR completes a task in #960.

To reproduce:

mamba create rv_1916
mamba activate rv_1916
pip install rastervision

rastervision predict \
https://s3.amazonaws.com/azavea-research-public-data/raster-vision/examples/model-zoo-0.20/isprs-potsdam-ss/model-bundle.zip \
https://s3.amazonaws.com/azavea-research-public-data/raster-vision/examples/model-zoo-0.20/isprs-potsdam-ss/sample-predictions/sample-img-isprs-potsdam-ss.tif \
prediction_output/ \
--channel-order 0 1 2

The above runs into an error complaining about rastervision_gdal_vsi being missing.

Checklist

  • Added needs-backport label if PR is bug fix that applies to previous minor release
  • Ran scripts/format_code and committed any changes
  • Documentation updated if needed
  • PR has a name that won't get you publicly shamed for vagueness

Notes

N/A

Testing Instructions

  • It should be possible to run the command above without errors.

The model bundles record every rastervision plugin that was installed when they were created and it was required to have all those plugins installed when using those bundles even if not actually needed. This change removes that requirement.
@AdeelH AdeelH added the needs-backport This PR needs to be backported to release branches label Sep 8, 2023
@AdeelH AdeelH mentioned this pull request Sep 8, 2023
9 tasks
@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Merging #1916 (c922821) into master (32eb656) will decrease coverage by 0.02%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #1916      +/-   ##
==========================================
- Coverage   81.57%   81.56%   -0.02%     
==========================================
  Files         190      190              
  Lines        9282     9287       +5     
==========================================
+ Hits         7572     7575       +3     
- Misses       1710     1712       +2     
Files Changed Coverage Δ
...tervision_pipeline/rastervision/pipeline/config.py 79.61% <66.66%> (+0.01%) ⬆️

... and 1 file with indirect coverage changes

@AdeelH AdeelH merged commit 91a93c5 into azavea:master Sep 8, 2023
2 checks passed
@AdeelH AdeelH deleted the bundle_plugins branch September 8, 2023 18:27
AdeelH added a commit to AdeelH/raster-vision that referenced this pull request Sep 22, 2023
The model bundles record every rastervision plugin that was installed when they were created and it was required to have all those plugins installed when using those bundles even if not actually needed. This change removes that requirement.

Co-authored-by: Adeel Hassan <ahassan@element84.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-backport This PR needs to be backported to release branches
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant