Skip to content

MAINT: Cleanup compatibility code for pathlib - #17529

Merged
charris merged 1 commit into
numpy:masterfrom
eric-wieser:remove-Path-is-None
Oct 10, 2020
Merged

charris merged 1 commit into
numpy:masterfrom
eric-wieser:remove-Path-is-None

Conversation

@eric-wieser

Copy link
Copy Markdown
Member

Path can now never be none, and PurePath is not used and not in __all__.

`Path` can now never be none, and `PurePath` is not used and not in `__all__`.
@BvB93

BvB93 commented Oct 10, 2020

Copy link
Copy Markdown
Member

This may be beyond the scope of this PR, but do we even need the is_pathlib_path function?

The only place where it seems to be used is in memmap.__new__ and replacing it with an
unconditional call to either os.fspath or os.fsdecode will also take care of any pathlib.Path instances.

@eric-wieser

eric-wieser commented Oct 10, 2020 •

Copy link
Copy Markdown
Member Author

No we don't - but we probably can't remove things from this module without deprecating them first.

We should certainly change all the callers of these useless aliases.

@BvB93

BvB93 commented Oct 10, 2020

Copy link
Copy Markdown
Member

No we don't - but we probably can't remove things from this module without deprecating them first.

Isn't compat a private module though?

@eric-wieser

eric-wieser commented Oct 10, 2020 •

Copy link
Copy Markdown
Member Author

I don't think we ever made it clear what is and is not public - it doesn't have any underscores in it, so the default user may assume it public, even if that was never our intent. It's a thorny enough issue to definitely be out of scope for this PR.

The C compat header has similar issues.

@BvB93

BvB93 commented Oct 10, 2020

Copy link
Copy Markdown
Member

Fair enough, let's keep this PR as it is then.

@charris
charris merged commit 2866c83 into numpy:master Oct 10, 2020
@charris

charris commented Oct 10, 2020

Copy link
Copy Markdown
Member

Thanks Eric.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants