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

tests: avoid sys.path hacks #106

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

akx
Copy link
Contributor

@akx akx commented Jan 2, 2024

Don't manually hack sys.path in tests/__init__.py; instead do this. (pip install -e . before running tests will have the same end effect.)

Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you @akx!


Man, I hate python. All of these stupid hacks and configurations just to work around the lack of proper relative imports. In JS/TS, I can just import the source file I need and that's that. Trivially simple.

@RunDevelopment RunDevelopment merged commit 733e8f0 into chaiNNer-org:main Jan 2, 2024
8 checks passed
@akx
Copy link
Contributor Author

akx commented Jan 2, 2024

@RunDevelopment In JS/TS land, woe be you if you do import foo from '../node_modules/...' (as I've seen people do in the wild) and you end up with no types (and tsc lighting up), or the wrong module kind getting bundled, or multiple copies of the same module, etc., etc. It's not perfect on that side of the fence either.

Anyway, as mentioned, no hack would be necessary either if you run pip install -e ., so the package is installed in editable mode and is hooked up to your sys.path as a kind of a site-package. This was used to be called, back in the dark days, python setup.py develop.

@akx akx deleted the no-test-sys-path-hacks branch January 2, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants