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

IMPORT_PATH for pyckel #2137

Open
Dorsug opened this issue Jan 16, 2025 · 2 comments
Open

IMPORT_PATH for pyckel #2137

Dorsug opened this issue Jan 16, 2025 · 2 comments

Comments

@Dorsug
Copy link

Dorsug commented Jan 16, 2025

Is your feature request related to a problem? Please describe.
There is no way to specify import path when using the pyckel wraper because the NICKEL_IMPORT_PATH related code was implemented in the cli part of the rust app.

Describe the solution you'd like
A great step would be for the pyckel wrapper to honor the NICKEL_IMPORT_PATH environment variable.

And as a great bonus we could add an import optional argument for the pyckel.run function to be able to specify the import directory at runtime.

@yannham
Copy link
Member

yannham commented Jan 16, 2025

This should be pretty straightforward to implement, as pyckel manipulates a Program object that has an add_import_paths method taking a list of paths. I'm wondering how we should expose this on the Python side; maybe with an optional parameter?

I would be inclined to keep the python wrapper simple and avoid surprises by being too smart, such as automatically and silently honoring NICKEL_IMPORT_PATH. It's not obvious that this is a good default, depending on the use-case, so I would rather provide the knob on the run function and let the Python caller decided where to take those import paths from - potentially from NICKEL_IMPORT_PATH if they want to, but not necessarily.

@Dorsug
Copy link
Author

Dorsug commented Jan 17, 2025

Yes, an optional parameter is definitely the way to go for the python side, maybe named import_paths to stay consistent with the rust api.

Okay, it makes sense why the automatic env variable isn’t the right fit.

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

No branches or pull requests

2 participants