Skip to content

BUG: Fix update notice when idc-index-data has a new version#72

Open
Sunderlandkyl wants to merge 1 commit intoImagingDataCommons:mainfrom
Sunderlandkyl:idc-index-data-update
Open

BUG: Fix update notice when idc-index-data has a new version#72
Sunderlandkyl wants to merge 1 commit intoImagingDataCommons:mainfrom
Sunderlandkyl:idc-index-data-update

Conversation

@Sunderlandkyl
Copy link
Collaborator

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.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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-data to the list of required libraries checked for updates
  • Updated installation logic to explicitly upgrade idc-index-data when updating packages
  • Implemented module reloading using importlib to load newly installed versions without requiring a restart

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

@@ -1521,8 +1522,26 @@ def setupPythonRequirements(self, update=False):
userMessage = "The module requires idc-index python package, which will now be installed."
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
@fedorov
Copy link
Member

fedorov commented Dec 15, 2025

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
Copy link
Collaborator Author

As far as I can tell the --upgrade argument only applies to the first library, so "idc-index-data" wasn't being updated.
Should be good to go now.

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.
@fedorov
Copy link
Member

fedorov commented Dec 16, 2025

As far as I can tell the --upgrade argument only applies to the first library, so "idc-index-data" wasn't being updated.

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants