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, Pathlib everywhere. #12515

Open
Carreau opened this issue Aug 27, 2020 · 23 comments · Fixed by #12632 · May be fixed by #12641
Open

Pathlib, Pathlib everywhere. #12515

Carreau opened this issue Aug 27, 2020 · 23 comments · Fixed by #12632 · May be fixed by #12641
Labels
Hacktoberfest you want to participate to hacktoberfest ? Here is an easy issue.

Comments

@Carreau
Copy link
Member

Carreau commented Aug 27, 2020

There are many places where we could make use of Pathlib.

Look for any places that uses with open(...) and ask yourself:

  • is the argument a string ?
  • would it make sens to make it a Path(),
  • how far upstream in the code can I make it a Path.

Don't try to bite more than you can chew (or more than I can review), try to fix 1 place at a time.

@Carreau Carreau added good first issue help wanted Hacktoberfest you want to participate to hacktoberfest ? Here is an easy issue. labels Aug 27, 2020
@Carreau
Copy link
Member Author

Carreau commented Aug 27, 2020

for example see #12512

@Inception95
Copy link
Contributor

Hi @Carreau, if no one is working on /core/page.py for using Pathlib, can I take this part?

@Carreau
Copy link
Member Author

Carreau commented Sep 21, 2020

Hi @Carreau, if no one is working on /core/page.py for using Pathlib, can I take this part?

Sure, please. Thanks.

@barhenkro
Copy link
Contributor

Hi, I would like to work on core/interactiveshell.py and add there more use of Pathlib.
Can I help with that?

@nicksspirit
Copy link
Contributor

Hey @Carreau can I take on testing/iptestcontroller.py and core/magics/execution.py ?

@MrMino
Copy link
Member

MrMino commented Oct 11, 2020

Might also be of use: https://docs.python.org/3/library/pathlib.html#correspondence-to-tools-in-the-os-module

There are many places other than with open() that might use pathlib treatment:

-    conda_history = os.path.join(sys.prefix, 'conda-meta', 'history')
-    return os.path.exists(conda_history)
+    return Path(sys.prefix, 'conda-meta', 'history').exists()

@nicksspirit
Copy link
Contributor

nicksspirit commented Oct 11, 2020 via email

@MrMino
Copy link
Member

MrMino commented Oct 11, 2020

Might also be of use: https://docs.python.org/3/library/pathlib.html#correspondence-to-tools-in-the-os-module

There are many places other than with open() that might use pathlib treatment:

Although... some of them might be less trivial and more error prone, especially when they're interpreted as text. For example, when returning a PosixPath object breaks code in the scope above because it is being ' '.join()-ed with other shell arguments.

@squarebat
Copy link

I am new to open source contribution and would love to work on this issue. After I am done with my changes, what would be the easiest way to test the code before I create a PR?

@davbrito
Copy link

davbrito commented Nov 5, 2020

I am new to open source contribution and would love to work on this issue. After I am done with my changes, what would be the easiest way to test the code before I create a PR?

Hello @squarebat
You should do this:

  • Install the testing dependencies this by running this command in the root of the repo:
$ pip3 install -e .[test]
  • Then execute the tests with
$ iptest

@squarebat
Copy link

I want to work on the test_run.py file. However, I'm having trouble resolving some errors in that file. The two main errors are

  • undefined variable get_ipython (This one is resolved by adding an import but that seems wrong)

  • undefined variable _ip : In some functions _ip is initialized as _ip = get_ipython() hence defining it, but in other functions it not initialized, hence causing error.

I have set up my environment using pip install .
I just wanted to know if I made a mistake while setting up the environment, or if there is a bug in the test_run.py file.

@davbrito
Copy link

davbrito commented Nov 7, 2020

I have set up my environment using pip install

You have to use pip install -e .[test]

The -e means that you Install it in editable mode.

@joesinghh
Copy link

Hello! This is my first time contributing to an open source project , I would like to work on this issue .
can i get assigned ?

ChandanChainani added a commit to ChandanChainani/ipython that referenced this issue Oct 13, 2022
@nitin-pandita
Copy link

Hey, I would love to work on this issue, as a beginner in Open source I think I can make it work. Thank you

Carreau added a commit that referenced this issue Oct 27, 2023
Since script `release` is meant to be run only when doing the actual
release of ipython, I could not easily verify it works correctly.
So please double check if it still works the same way as before.

Related to: #12515
Carreau added a commit that referenced this issue Oct 27, 2023
* Replace `os.path` with `pathlib.Path`
* Add type hints for the functions

Related to: #12515
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest you want to participate to hacktoberfest ? Here is an easy issue.
Projects
None yet