Skip to content

Add backwards compatibility handler for old input_structure trait#1423

Merged
edan-bainglass merged 1 commit intoaiidalab:mainfrom
edan-bainglass:back-comp-fix
Nov 26, 2025
Merged

Add backwards compatibility handler for old input_structure trait#1423
edan-bainglass merged 1 commit intoaiidalab:mainfrom
edan-bainglass:back-comp-fix

Conversation

@edan-bainglass
Copy link
Copy Markdown
Member

This PR adds handling of remaining usage of the old input_structure trait, routing to the new structure_uuid trait.

# BACKWARDS COMPATIBLE - remove when all plugins are updated!
warnings.warn(
(
"The `input_structure` dependency is deprecated. "
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I will include like Plugin developer: ... so the user doesnt get confused with this warning

Copy link
Copy Markdown
Member Author

@edan-bainglass edan-bainglass Nov 22, 2025

Choose a reason for hiding this comment

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

When will the user ever see this? These will only show up in tests, or if you switch to edit mode. They won't show in app mode. Note that we do not do "Plugin developer: ..." in any of our warnings. Have users raised this concern?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ahhh I thought that this will be the warning like the ones that Xing used to put , my bad

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Which ones are those? 😅

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Also, let me know if anything is holding this PR back 🙏

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nothing (holding this PR), is good , is just that Xing used to put warnings in the app mode, for example some can appear in the Calculation History, is there is an incompatibility or something with the code

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I see. I just tested the vibroscopy plugin (which is incompatible). No warnings shown on install, load, usage, etc. Great if you can approve, so we can merge and proceed 🙏 If for some unforeseen reason, the message shows up in a user interface, we can iterate.

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 22, 2025

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.18%. Comparing base (51933a0) to head (a506662).
⚠️ Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
src/aiidalab_qe/common/mixins.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1423   +/-   ##
=======================================
  Coverage   72.18%   72.18%           
=======================================
  Files         108      108           
  Lines        7259     7263    +4     
=======================================
+ Hits         5240     5243    +3     
- Misses       2019     2020    +1     
Flag Coverage Δ
python-3.11 72.17% <60.00%> (-0.02%) ⬇️
python-3.9 72.21% <60.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@edan-bainglass
Copy link
Copy Markdown
Member Author

@AndresOrtegaGuerrero note that the failed test is unrelated. This is being handled by #1337, which is now rebased on this due to it failing for plugin incompatibility.

@edan-bainglass
Copy link
Copy Markdown
Member Author

@AndresOrtegaGuerrero can I merge this?

Copy link
Copy Markdown
Member

@AndresOrtegaGuerrero AndresOrtegaGuerrero left a comment

Choose a reason for hiding this comment

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

LGTM thank you!

@AndresOrtegaGuerrero
Copy link
Copy Markdown
Member

@AndresOrtegaGuerrero can I merge this?

Yes sorry i got busy on other things, thanks!

@edan-bainglass edan-bainglass merged commit 4c5ddf8 into aiidalab:main Nov 26, 2025
8 of 9 checks passed
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