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.

Sign up for GitHub

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

Jump to bottom

use pathlib for paths #2057

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

use pathlib for paths #2057

wants to merge 6 commits into from

Conversation

danieleades
Copy link
Contributor

No description provided.

@danieleades
Copy link
Contributor Author

mypy coverage still not good enough to do this effectively

@danieleades
Copy link
Contributor Author

having another go now mypy coverage is higher

@danieleades danieleades force-pushed the pathlib branch 6 times, most recently from b8732b9 to 24f099e Compare April 22, 2024 13:43
@danieleades
Copy link
Contributor Author

CI failures due to #2063

@danieleades danieleades force-pushed the pathlib branch 2 times, most recently from a5761cc to 0a7bc8e Compare April 25, 2024 12:29
@jensens jensens added the patch Little bugfix, typo fix, etc. When only patch version should be updated label Apr 25, 2024
@danieleades danieleades force-pushed the pathlib branch 13 times, most recently from d9c24bb to e38d984 Compare May 6, 2024 09:32
@danieleades danieleades force-pushed the pathlib branch 3 times, most recently from 31d5afa to 521c8b4 Compare May 7, 2024 06:30
@danieleades
Copy link
Contributor Author

@jensens this is getting pretty close to ready.

I will need some help to go back through this PR and ensure that I haven't made breaking changes to the API.
For example, I've changed some signatures from str to Path, which is fine for internal APIs but a breaking change for public APIs. There are at least some instances of this to be addressed. Can you help me spot them?

My intention is to change them to Path | str, and add a deprecation warning for uses of str, so that they can be changed to Path in a future major release

@jensens
Copy link
Contributor

jensens commented May 7, 2024

@danieleades Great! But ... Ok, this is kind of massive. I need to take some time to review all this. I'll come back in the next days, no time today or tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
patch Little bugfix, typo fix, etc. When only patch version should be updated
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@danieleades @jensens