Add warning about set private attr when using proxy #5209
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5209 +/- ##
==========================================
- Coverage 88.82% 88.82% -0.01%
==========================================
Files 574 574
Lines 48912 48929 +17
==========================================
+ Hits 43446 43459 +13
- Misses 5466 5470 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Looks great! Except for the duplicated commented code.
Also, thanks for splitting this. Am happy to bring this into v0.4.17 and will probably remove #5195 from the release.
napari/utils/_proxies.py
Outdated
| # raise AttributeError( | ||
| # trans._( | ||
| # "Private attribute set ('{typ}.{name}') not allowed in this context.", | ||
| # deferred=True, | ||
| # name=name, | ||
| # typ=typ, | ||
| # ) | ||
| # ) |
There was a problem hiding this comment.
I'm not sure why the commented code is in the original (I'd actually love to delete it), but we definitely shouldn't add a repeat of it without an explanation of why commented code is on the main branch.
There was a problem hiding this comment.
I'm feeling mildly destructive today, so if you also feel like deleting the original commented code, please go ahead.
There was a problem hiding this comment.
If I correctly remember this is just preparation on moment where we remove private access. Just remove warning and uncomment exception.
They have a little different text as first is about getting value and second about setting it.
But maybe proper solution will be prepare one text and move it to one function?
Personally I do not understand why thi text is translated as it is not connected with user action.
|
I do not understand the perf test faliture. |
Neither do I. I can't reproduce it locally (on macOS) either. Though I haven't tried pyside2. Not sure why this PR would trigger that failure, though it seems suspicious that it failed on two distinct runs with different Python versions. Have you tried rerunnning? The failing test does two things that I don't love: starts a subprocess and waits a seemingly arbitrary amount of time (100 milliseconds) for something to occur. I'd be OK temporarily disabling it. |
I know what happens in case of PySide6, and it is unrelated to this code. It is connected with a new release of PySide6. |
The failures here are associated with (ubuntu-latest, Python 3.8/3.10, pyside2), not pyside6. There has not been a recent release of pyside2, so that doesn't directly explain it. |
|
this pyside2 is also performance |
|
I reran the failed checks and they passed. I also pulled down this branch and tried running with Therefore, I will merge this after 24 hours unless there are any objections. |
* add warning about set private attr when using proxy * add commented exception * extract _is_called from napari * Move prepared raise
* add warning about set private attr when using proxy * add commented exception * extract _is_called from napari * Move prepared raise
* add warning about set private attr when using proxy * add commented exception * extract _is_called from napari * Move prepared raise
* add warning about set private attr when using proxy * add commented exception * extract _is_called from napari * Move prepared raise
* add warning about set private attr when using proxy * add commented exception * extract _is_called from napari * Move prepared raise
* add warning about set private attr when using proxy * add commented exception * extract _is_called from napari * Move prepared raise
* add warning about set private attr when using proxy * add commented exception * extract _is_called from napari * Move prepared raise
Description
extracted from #5195
Type of change
References
How has this been tested?
as there are small differences between the two Qt bindings.
Final checklist:
trans.to make them localizable.For more information see our translations guide.