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

[ENH] Edit Domain: Change types of multiple variables #6426

Merged
merged 3 commits into from
Jun 29, 2023

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Apr 24, 2023

Issue

Fixes #6088.

Based on #6415 to avoid conflicts.

Description of changes

Allows changing the variable type for multiple variables at once.

As discussed with @markotoplak, editing other properties, even labels, would be confusing because controls would have to be disabled if selected variables differ in those properties. Also, the widget already has over 3000 lines of complex code.

The only feature we could easily add would be a tri-state checkbox for unlinking.

The widget has four separate editors for different variable types, which are stacked into a single editor. This PR implements a fifth editor, which is blank (except for the combo with type) and can handle multiple variables. Existing editors are kept as they are, that is, they accept a single variable. The "top editor" has separate methods for single and multiple variables. One might think that the four editors could accept lists instead of single variables to avoid having duplicated methods in the top editor. I thought so for long time, too, but the code was just getting more and more complex. This solution is simpler.

Includes
  • Code changes
  • Tests
  • N/A Documentation

@janezd janezd changed the title Edit domain multiple Edit Domain: Change types of multiple variables Apr 24, 2023
@janezd janezd changed the title Edit Domain: Change types of multiple variables [ENH] Edit Domain: Change types of multiple variables Apr 24, 2023
@janezd janezd force-pushed the edit-domain-multiple branch from f420bc6 to 160abf0 Compare April 25, 2023 20:50
@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Merging #6426 (6ee8479) into master (78a8d71) will increase coverage by 0.02%.
The diff coverage is 97.45%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6426      +/-   ##
==========================================
+ Coverage   87.63%   87.66%   +0.02%     
==========================================
  Files         321      321              
  Lines       69254    69374     +120     
==========================================
+ Hits        60693    60816     +123     
+ Misses       8561     8558       -3     

@janezd janezd force-pushed the edit-domain-multiple branch from 160abf0 to 3a86a90 Compare May 1, 2023 18:31
@janezd janezd marked this pull request as ready for review May 1, 2023 18:31
@janezd janezd force-pushed the edit-domain-multiple branch 2 times, most recently from 5742454 to 83ef78e Compare May 1, 2023 20:07
@markotoplak
Copy link
Member

/rebase

@markotoplak markotoplak force-pushed the edit-domain-multiple branch from 83ef78e to 0485d6a Compare May 26, 2023 09:13
@markotoplak
Copy link
Member

@janezd, just to double check: if I understood this code correctly, it does not in any way change how settings are stored (because they are always saved per variable), and we thus do not need to increase the settings version.

@markotoplak markotoplak merged commit 2825cbf into biolab:master Jun 29, 2023
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.

Edit Domain: enable setting the type for several variables at once
2 participants