avoid importing ArrayDataModel if numpy is not installed #749
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kitchoi
reviewed
Oct 13, 2020
| try: | ||
| from pyface.data_view.data_models.api import ArrayDataModel | ||
| except TraitError: | ||
| pass |
There was a problem hiding this comment.
In this case I think the Look-Before-You-Leap is better, because the TraitError is such an overloaded exception and I would want to be sure that we are only excusing for the scenario when numpy is not available.
requires_numpy actually checks if np (imported from optional_dependencies) is None, so doing if np is not None before importing ArrayDataModel would match the logic of requires_numpy and is more explicit about the fact that we are expecting NumPy not being available, not anything else that could cause TraitError.
Really TraitError should be reserved for validation error but it has been used for so many other purposes, and changing that would break so much code...
|
I will label this PR for backporting to the maint/7.1 near the time when we make the release. |
kitchoi
added
the
need backport to 7.1
Codecov Report
@@ Coverage Diff @@
## master #749 +/- ##
==========================================
+ Coverage 40.31% 40.85% +0.54%
==========================================
Files 502 506 +4
Lines 27652 27813 +161
Branches 4193 4217 +24
==========================================
+ Hits 11148 11364 +216
+ Misses 16010 15976 -34
+ Partials 494 473 -21
Continue to review full report at Codecov.
|
33 tasks
kitchoi
pushed a commit
that referenced
this pull request
Oct 19, 2020
* avoid importing ArrayDataModel if numpy is not installed * replace try except with look-before-you-leap approach * typo (cherry picked from commit a68eb28)
kitchoi
added a commit
that referenced
this pull request
Oct 19, 2020
* Add a section to the documentation on the basics of Pyface Widgets. (#739) (cherry picked from commit 86c9a9c) * avoid importing ArrayDataModel if numpy is not installed (#749) * avoid importing ArrayDataModel if numpy is not installed * replace try except with look-before-you-leap approach * typo (cherry picked from commit a68eb28) * Add api.py to data view packages (#750) * Add/update api.py modules for data views. * Use data view api modules in examples. * Add data view api modules to documentation examples. * Add some extra imports, change import locations. * Test API modules in pyface.data_view (#752) * Add test for pyface.data_view.api * Add test for pyface.data_view.exporters.api * Add test for data models API Make sure the api module is importable even if we don't have NumPy * Add tests to help developers keeping the list up-to-date * Fix language in a comment * Remove mixins from .api. * Adjust tests. Co-authored-by: Kit Choi <[email protected]> (cherry picked from commit e9b7d96) * Mention the data view API being provisional in the User Manual (#758) (cherry picked from commit cfb3f8c) * Set comparison mode to identity for MDataViewWidget.exporters (#759) (cherry picked from commit d5f5e7f) * Skip tests packages in API documentation (#761) (cherry picked from commit 9f13097) * Skip load_tests in API documentation (#762) * Skip load_tests in autodoc * Local flake8 (cherry picked from commit a3ad0f2) * Additional documentation for the DataView (#763) * Add documentation on Drag and Drop. * Add a note about DataViewGetError. * More notes on value types. * Apply suggestions from code review Co-authored-by: Kit Choi <[email protected]> * Fix references to exporters. Co-authored-by: Kit Choi <[email protected]> (cherry picked from commit 3acd644) * FIX docs click command to work on windows (#764) * FIX : Docs build on windows modified: etstool.py * Update etstool.py Co-authored-by: Kit Choi <[email protected]> Co-authored-by: Kit Choi <[email protected]> (cherry picked from commit bac64de) * CLN : Rename api-docs click command to docs (#766) Now, traitsui and pyface have the same command name - reducing unnecessary cognitive overhead. modified: etstool.py (cherry picked from commit 3a6d349) * FIX : Correct a docstring (#767) (cherry picked from commit 95d0474) * Get ModalDialogTester tests working on pyqt5 (#768) * FIX : Get ModalDialogTester tests working on pyqt5 The tests were previously being skipped because they failed mysteriously on pyqt5 - on further investigation, I saw weird errors related to two of the tests - a missing AttributeError for self.gui modified: pyface/ui/qt4/util/tests/test_modal_dialog_tester.py * FIX : Reverse order of superclasses modified: pyface/ui/qt4/util/tests/test_modal_dialog_tester.py (cherry picked from commit 96b8580) * Address sphinx warnings when building docs (#769) * FIX : Address sphinx warnings when building docs * FIX : Revert a confusing change (i couldnt come up with a worse commit message) modified: pyface/timer/do_later.py (cherry picked from commit 5b21b49) * Explicitly extract data from bytearrays rather than rely on wrapper. (#773) (cherry picked from commit 0d5565f) * Update changelog Co-authored-by: Corran Webster <[email protected]> Co-authored-by: aaronayres35 <[email protected]> Co-authored-by: Poruri Sai Rahul <[email protected]>
kitchoi
removed
the
need backport to 7.1
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #742
In this PR I simply catch the error that occurs when trying to import
ArrayDataModelwithoutnumpybeing installed. The tests themselves all are decorated with@requires_numpy, so they aren't run regardless. This change simply prevents the errors we were seeing in issue 742.This is clearly a work around and does not solve the root of the problem. See related comment: #740 (comment)
Also I am not sure if this PR should be based on top of master or
maint/7.1. I will happily change if needed.