os.PathLike support for Directory traits
#867
Conversation
- also added unit tests
Codecov Report
@@ Coverage Diff @@
## master #867 +/- ##
=========================================
Coverage ? 72.49%
=========================================
Files ? 51
Lines ? 6370
Branches ? 1278
=========================================
Hits ? 4618
Misses ? 1359
Partials ? 393
Continue to review full report at Codecov.
|
| @@ -1496,6 +1500,12 @@ def validate(self, object, name, value): | |||
|
|
|||
| Note: The 'fast validator' version performs this check in C. | |||
| """ | |||
| if fspath is not None: | |||
There was a problem hiding this comment.
It may be worth a comment explaining the need for the if, and indicating that the test can be removed once we drop Python 3.5 support. Alternatively, such a comment could go up at the top of the file, where the fspath import is attempted.
traits/trait_types.py
Outdated
| @@ -1516,6 +1526,9 @@ def create_editor(self): | |||
| class Directory(BaseDirectory): | |||
| """ A fast-validating trait type whose value is a directory path string. | |||
|
|
|||
| For Python 3.6 and greater, it also accepts objects implementing | |||
| the `os.PathLike` interface; converting them to corresponding string. | |||
There was a problem hiding this comment.
Grammar nit: replace the semicolon with a comma here (and perhaps remove the first comma in the sentence). Add "the" before "corresponding string".
| # Define the C-level fast validator to use if the directory existence | ||
| #: test is not required: | ||
| if not exists: | ||
| self.fast_validate = (ValidateTrait.coerce, str) | ||
| super(Directory, self).__init__( |
There was a problem hiding this comment.
We should probably have a comment indicating that fast validation is disabled. Ideally, also open an issue on the tracker to remind us to decide whether to implement fast validation or not.
traits/tests/test_directory.py
Outdated
| with self.assertRaises(TraitError): | ||
| foo.path = __file__ | ||
|
|
||
| @unittest.skipIf(sys.version_info < (3, 6), TESTS_SKIPPED_MESSAGE) |
There was a problem hiding this comment.
Suggestion: you could reduce repetition and improve readability here by creating a single decorator at the top of the module. For example (untested):
requires_fspath = unittest.skipIf(..., "Test requires os.fspath, which is unavailable before Python 3.6")
Co-Authored-By: Mark Dickinson <mdickinson@enthought.com>
|
@mdickinson |
Co-Authored-By: Mark Dickinson <mdickinson@enthought.com>
Co-Authored-By: Mark Dickinson <mdickinson@enthought.com>
contributes to #851
The fast validation for. I will create separate PR for the C level validation.Directorytrait is done in C only when existence check is not required. Otherwise the validation is done in Python by calling base class validation