search

jazzband / pathlib2

Backport of pathlib aiming to support the full stdlib Python API.
MIT License
81 stars 31 forks source link

Refactoring functions and Fix Code Expression #73

Closed yezz123 closed 2 years ago

yezz123 commented 3 years ago

Hello @mcmtroffaes 👋🏻 I just refactor some functions in the code itself and edit the test relate to testing the pathlib.

mcmtroffaes commented 3 years ago

Wow, thanks for the contribution, I very much appreciate your enthusiasm! However, there are two issues:

  1. Many parts of your patch break 2.7 compatibility (as shown by the regression testing). So at least for now I suggest holding off on this, until it's clear whether or not another 2.7 release needs to be made (I think not but I don't want to jump the gun before allowing a wider discussion). See #71.
  2. So far, the spirit of the library has been to keep a minimal delta with upstream cpython in order to make it as easy as possible to track upstream. Whence, to modernize for Python 3, I think it would be less work to simply make a fresh copy from cpython pathlib, and then ensure compatibility with Python 3.6+. In particular, any refactoring the code should ideally happen in cpython pathlib, and not in pathlib2 here.
yezz123 commented 3 years ago

@mcmtroffaes I agree with the same idea in this issue #71!

As of January 1st, 2020 no new bug reports, fixes, or changes will be made to Python 2, and Python 2 is no longer supported.

mcmtroffaes commented 3 years ago

Also, about the idea of refactoring the default pathlib in CPython, this gonna be somehow hard and need a full review in multiple environments, maybe also changing the language pragmatics, and I guess this is why Pathlib 2 is here to give a great addon with more features than the default one.

Yes, going forward, maybe that could be a future role for pathlib2 if that's desired and agreed. However, I'm definitely not the right person to shepherd such developments. The point I want to get across is that historically that's not been the case and it's been an explicit "goal" of the project not to add features that weren't in cpython: pathlib has been the upstream for pathlib2, not the other way around.

graingert commented 3 years ago

Also, about the idea of refactoring the default pathlib in CPython, this gonna be somehow hard and need a full review in multiple environments, maybe also changing the language pragmatics, and I guess this is why Pathlib 2 is here to give a great addon with more features than the default one.

Yes, going forward, maybe that could be a future role for pathlib2 if that's desired and agreed. However, I'm definitely not the right person to shepherd such developments. The point I want to get across is that historically that's not been the case and it's been an explicit "goal" of the project not to add features that weren't in cpython: pathlib has been the upstream for pathlib2, not the other way around.

my 'vision' is for this project to generally follow the example of contextlib2, eg just copy/paste updates from cpython's 'main' branch and have a minimal diff to make it work on the oldest supported (currently py3.6+) python

Historically contextlib2 was the upstream and stdlib contextlib was the downstream, but this has now mostly reversed see https://github.com/jazzband/contextlib2/issues/31

mcmtroffaes commented 3 years ago

Sounds good, @graingert.