Simplify Module Imports by Avoiding Modification of `sys.path`

shashank-iitbhu commented 6 months ago

Terms

[X] I have searched all open bug reports
[X] I agree to follow Scribe-Data's Code of Conduct

Behavior

Description

Currently, in several scripts within the Scribe-Data project, we are modifying sys.path to include the src/scribe_data directory to ensure that the scribe_data module can be imported correctly. For example,

PATH_TO_SCRIBE_ORG = os.path.dirname(sys.path[0]).split("Scribe-Data")[0]
PATH_TO_SCRIBE_DATA_SRC = f"{PATH_TO_SCRIBE_ORG}Scribe-Data/src"
sys.path.insert(0, PATH_TO_SCRIBE_DATA_SRC)

This can lead to potential issues with import resolution and is generally not considered a best practice.

Suggested Changes

Remove such modification of sys.path.
Directly use from scribe_data import module_name
Encouraging developers to install the package in editable mode during development by using the command pip install -e . from the project root directory. This allows for testing changes to the code without needing to reinstall the package.
Python's import system uses the sys.path list to determine where to look for modules to import. After we have built the modules using pip install -e ., the scribe_data module will already be present in sys.path as it already has path to pip packages and then we do not need to add src/scribe_data explicitly to sys.path.
Official Guidelines for working on a Python package in development mode. Link

shashank-iitbhu commented 6 months ago

I have already tested this locally. Let me know if these changes can help. Open for discussion. cc @andrewtavis @wkyoshida Should I open a PR, so that you can also test this?

andrewtavis commented 6 months ago

Makes sense to me, @shashank-iitbhu! Feel free to send along a PR 🚀 I'll send along the structure changes right now, so if you could bring those into your branch before that'd be great 😊

andrewtavis commented 6 months ago

2b72e64 just sent along the directory structure update, @shashank-iitbhu, so you're good to pull in the changes and send along a PR for this :)

shashank-iitbhu commented 6 months ago

Opened a PR for these changes. @andrewtavis This I believe highlights the need to include instructions for using pip install -e . in our contributing guide or documentation.

Jk40git commented 6 months ago

@andrewtavis and @shashank-iitbhu please is this issue still opened?

shashank-iitbhu commented 6 months ago

@andrewtavis and @shashank-iitbhu please is this issue still opened?

I have linked a PR above, haven't been merged yet. It's still under review.

andrewtavis commented 6 months ago

Closed in #123 🚀 Thank you, @shashank-iitbhu!

scribe-org / Scribe-Data