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

DataView Color clean-up #709

Merged
merged 1 commit into from
Sep 30, 2020
Merged

Conversation

corranwebster
Copy link
Contributor

This PR does two things:

We will probably revisit ColorValue again once there are custom editors available.

@codecov-commenter
Copy link

codecov-commenter commented Sep 30, 2020

Codecov Report

Merging #709 into master will increase coverage by 0.76%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #709      +/-   ##
==========================================
+ Coverage   39.76%   40.53%   +0.76%     
==========================================
  Files         493      502       +9     
  Lines       27259    27614     +355     
  Branches     4140     4185      +45     
==========================================
+ Hits        10840    11193     +353     
- Misses      15944    15950       +6     
+ Partials      475      471       -4     
Impacted Files Coverage Δ
pyface/ui/qt4/data_view/data_view_item_model.py 49.30% <38.46%> (-0.70%) ⬇️
pyface/ui/wx/data_view/data_wrapper.py 86.36% <86.36%> (ø)
pyface/data_view/i_data_wrapper.py 88.46% <88.46%> (ø)
...face/data_view/data_models/row_table_data_model.py 91.25% <91.25%> (ø)
pyface/ui_traits.py 86.11% <93.93%> (+2.32%) ⬆️
pyface/data_view/value_types/color_value.py 95.45% <95.45%> (ø)
pyface/color.py 100.00% <100.00%> (ø)
pyface/data_view/abstract_value_type.py 97.72% <100.00%> (+0.29%) ⬆️
pyface/data_view/data_models/data_accessors.py 100.00% <100.00%> (ø)
pyface/data_view/data_wrapper.py 100.00% <100.00%> (ø)
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 946a9c6...91472e1. Read the comment docs.

@corranwebster corranwebster merged commit c98015c into master Sep 30, 2020
@corranwebster corranwebster deleted the fix/data-view-pyface-color-trait branch September 30, 2020 10:36
@kitchoi
Copy link
Contributor

kitchoi commented Sep 30, 2020

Please could we consider the pattern of splitting Str and CStr so that one trait type does strictly validation and the other does casting. I believe the casting capability in the color trait is motivated by migrating legacy code in enable amd chaco. The fact that one can assign primitive types (e.g. str) to an attribute to get an instance back makes tracking such usage in enable/chaco for refactoring difficult. (It is also surprising to read.) By having a separate trait type that doesn't do casting, we can prevent this anti-pattern cropping up in new code. Note that I am only talking about the 'validate' method on the trait type, not the instantiation. Accepting string in the trait type instantiation is fine.

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.

Fix DataView ColorValue expected test failures
4 participants