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.

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

Pathlib in core.magics #12615

Merged
merged 4 commits into from Oct 13, 2020
Merged

Pathlib in core.magics #12615

merged 4 commits into from Oct 13, 2020

Conversation

MrMino
Copy link
Member

@MrMino MrMino commented Oct 11, 2020

See #12515.

This introduces usage of pathlib, including in places where it could be used instead of os.path, in:

  • IPython.core.magics.execution
  • IPython.core.magics.packaging

Don't be scared by the amount of edited lines. By the boy-scout rule I've fixed some issues with code style that my linter was yapping about: redundant or missing whitespace / lines, wrong indentation, unnecessary usage of \. These changes shouldn't alter any behavior. In IPython.core.magics.execution it turned out to be a lot of hunks.

@MrMino MrMino force-pushed the pathlib_in_core_magics branch 2 times, most recently from 5e72330 to eba2b69 Compare October 11, 2020 01:40
@MrMino
Copy link
Member Author

MrMino commented Oct 11, 2020

Turns out I had to move the code style changes out, because darkening them is a whole other big can of worms. They now live here. Not sure if I should PR partial style changes.

Also, I decided not to add the following change, as it didn't visibly improve anything:

IPython/core/magics/execution.py:
-            args = shellglob(map(os.path.expanduser,  arg_lst[1:]))
+            args = shellglob(map(lambda p: str(Path(p).expanduser()), arg_lst[1:]))

I wonder if IPython.utils.path.shellglob still adds value, given it was implemented back in 2012.

@Carreau Carreau added this to the 8.0 milestone Oct 13, 2020
@Carreau
Copy link
Member

Carreau commented Oct 13, 2020

Don't worry f the code formatting is too-agressive, we can disable it.

This looks great ! Thanks !

@Carreau Carreau merged commit 9996a0a into ipython:master Oct 13, 2020
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.

None yet

2 participants