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

Typed settings #4721

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

Typed settings #4721

wants to merge 8 commits into from

Conversation

janezd
Copy link
Contributor

@janezd janezd commented May 1, 2020

Issue

Uses changes from biolab/orange-widget-base#62.

Changes in projections are given as an example of kinds of annotations that will be required from widgets to avoid warnings and allow storing their settings as json.

Includes
  • Code changes
  • Tests
  • Documentation

@codecov
Copy link

codecov bot commented May 2, 2020

Codecov Report

Merging #4721 (af8cbc4) into master (343f80b) will decrease coverage by 2.55%.
The diff coverage is 90.00%.

@@            Coverage Diff             @@
##           master    #4721      +/-   ##
==========================================
- Coverage   86.24%   83.68%   -2.56%     
==========================================
  Files         300      275      -25     
  Lines       61027    55609    -5418     
==========================================
- Hits        52634    46539    -6095     
- Misses       8393     9070     +677     

@janezd janezd changed the title [WIP] [RFC] Setting type checking in Domain and ClassValues context handlers; annotations in projections [WIP] Setting type checking in Domain and ClassValues context handlers; annotations in projections Sep 4, 2020
@janezd janezd marked this pull request as draft September 24, 2020 15:58
@janezd janezd changed the title [WIP] Setting type checking in Domain and ClassValues context handlers; annotations in projections Setting type checking in Domain and ClassValues context handlers; annotations in projections Sep 24, 2020
@janezd janezd changed the title Setting type checking in Domain and ClassValues context handlers; annotations in projections Setting type checking in Domain and ClassValues context handlers; annotations in some widgets Oct 3, 2020
if version < 3:
sel = context.values["selection"]
context.values["selection"] = ([(name, vtype + 100)
for name, vtype in sel], -3)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This migration is removed because annotated settings are no longer encoded into tuples. DomainContextHandler thus performs an exactly opposite migration for all annotated settings. If this migration here is left in, it would "unmigrate" the setting.

I assume such cases like this will be rare. So although this situation could be avoided if DomainContextHandler performed the migration after calling migrate_context, the downside would be that this would force the future migrate_context to work with raw, encoded settings instead of simpler, decoded, and thus force them to "understand" how coding is done - which should be of no concern to migrations.

@janezd janezd changed the title Setting type checking in Domain and ClassValues context handlers; annotations in some widgets Typed settings Oct 30, 2020
@janezd janezd force-pushed the typed-settings branch 3 times, most recently from 4c3c8a5 to 75489ff Compare December 4, 2020 15:29
@janezd janezd added the needs discussion Core developers need to discuss the issue label Mar 11, 2021
@janezd janezd removed the needs discussion Core developers need to discuss the issue label Mar 14, 2021
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.

1 participant