BUG: Fix update notice when idc-index-data has a new version #72
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where idc-index-data was being marked as outdated but was not actually being upgraded, causing persistent update notifications. The fix ensures that both idc-index and idc-index-data are properly tracked for updates and upgraded when needed, with module reloading to ensure the new versions are used immediately.
Key Changes:
- Added
idc-index-datato the list of required libraries checked for updates - Updated installation logic to explicitly upgrade
idc-index-datawhen updating packages - Implemented module reloading using
importlibto load newly installed versions without requiring a restart
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sunderlandkyl
force-pushed
the
idc-index-data-update
branch
2 times, most recently
from
83c71c3 to
582296f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| userMessage = "The current idc-index python package is out of date, and will now be updated." | ||
| errorMessage = f"Failed to {'install' if needToInstall else 'update'} idc-index." | ||
| if needToInstall: | ||
| userMessage = "The module requires idc-index python package, which will now be installed." |
There was a problem hiding this comment.
The user message references only 'idc-index python package' but the code now installs/updates both 'idc-index' and 'idc-index-data' packages. Consider updating this message to mention both packages for clarity.
|
Oh wow! But I am surprised - I was pretty sure when idc-index is upgraded, it will pull in upgraded idc-index-data, and I am pretty sure it worked for me that way. In any case, this seems to be harmless, so I definitely can merge and we can discuss when we meet tomorrow. Or let me know and I can wait until we discuss tomorrow. |
Sunderlandkyl
force-pushed
the
idc-index-data-update
branch
from
582296f to
9deb17f
Compare
|
As far as I can tell the --upgrade argument only applies to the first library, so "idc-index-data" wasn't being updated. |
idc-index-data would be marked as out of date, however it would not be upgraded, making it impossible to get rid of the update required message. Fixed by calling --upgrade on idx-index-data to trigger the update. Also reload the idc-index library after updating so that the new library is used.
Sunderlandkyl
force-pushed
the
idc-index-data-update
branch
from
9deb17f to
fcd035b
Compare
Right, but the dependency for idc-index is always updated to specify new version of idc-index-data. So for example the latest idc-index specifically requests idc-index-data 23.0.1: https://github.com/ImagingDataCommons/idc-index/blob/main/pyproject.toml#L35 |
2 participants
idc-index-data would be marked as out of date, however it would not be upgraded, making it impossible to get rid of the update required message. Fixed by calling --upgrade on idx-index-data to trigger the update.
Also reload the idc-index library after updating so that the new library is used.