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

DEV: Can pydevtool.cli be used to create a unified context? #45

Closed
ganesh-k13 opened this issue Feb 26, 2023 · 14 comments
Closed

DEV: Can pydevtool.cli be used to create a unified context? #45

ganesh-k13 opened this issue Feb 26, 2023 · 14 comments

Comments

@ganesh-k13
Copy link
Contributor

ganesh-k13 commented Feb 26, 2023

Scipy uses pydevtool.cli that helps in bunching the common commands like build-dir. As of today in this repo, we are repeating build-dir in every command and lack a few utility commands to infer/set python paths from these common args.

I'd be happy to bunch it and refactor all the commands into a subclass of Task and add common inferencing functions like Dirs but wanted to confirm if this is the right way forward or the project maintainers had something else in mind.

@stefanv
Copy link
Member

stefanv commented Feb 26, 2023

For devpy, the design is intended to be as simple as possible, and to only use dependencies if truly necessary.

We'd have to see how it would compare to implement this outside pydevtool.cli, and also to ask whether the increased complexity is worth it.

@ganesh-k13
Copy link
Contributor Author

Ah ok, that makes sense. I was working on bench and had to use a small portion of build command and wanted to clarify this. I think I'll stick with what's there today.

@rgommers
Copy link
Contributor

After working on some NumPy code again today, I wasted bunch of time scratching my head why ./dev.py test was still failing when I thought I had fixed something. The lack of dependencies requires typing:

./dev.py build && ./dev.py test

all the time. I think the gap in usability is pretty large.

@stefanv
Copy link
Member

stefanv commented Feb 28, 2023

I'm not sure how to read your comment above. Can you clarify? Is it:

(1) The usability gap is large enough that I prefer to customize SciPy's dev.py for NumPy, or
(2) We should improve dev.py to close the gap in usability.

I suspect you are hinting at (1)? I don't mind if you want to go that route. I'll keep working on the tool either way since it's being used by a few other libraries too.

@rgommers
Copy link
Contributor

So far I've been waiting for (2). I think dependencies are necessary though when working on a numpy/scipy-sized code base, and it's hard to use devpy for numpy development right now. However, for smaller projects where (a) editing compiled code is rare and (b) that doesn't use Meson, devpy could rely on editable installs and then explicit dependencies between tasks may not be needed (pure Python edits get reflected immediately). In that case, we should do (1) for NumPy.

What do you think?

@stefanv
Copy link
Member

stefanv commented Feb 28, 2023

Are you happy to call the build command each time before testing, or do you want it to be smart about detecting when a build is necessary? I think the first scenario I can complete without much difficulty.

@rgommers
Copy link
Contributor

There's no need to be smart - a do-nothing rebuild takes only about 100 ms. Also, any commit updates at least the version number when that includes a git hash, so a rebuild is very often needed and checking that is difficult.

@stefanv
Copy link
Member

stefanv commented Feb 28, 2023

OK, cool, I'll get this added later today (sorry for the wasted time, that is never good!). I was trying to figure out how one would pass options through to the build via the test command, but I see SciPy's dev.py doesn't allow for that either. You have to run the dedicated build command if you want to specify flags like --gcov, but the test command itself just invokes a basic build.

@rgommers
Copy link
Contributor

No worries. And yes, it's nothing too complicated.

@stefanv
Copy link
Member

stefanv commented Mar 1, 2023

Please see #51

@ganesh-k13
Copy link
Contributor Author

ganesh-k13 commented Mar 1, 2023

I think it makes sense for simplicity but I hope we don't repeat what we have at SciPy. In reference to dependencies, Task object provides task_dep that does a topo sort of dependencies.
[EDIT] Or at least I hope it does :), seeing the code for pydevtools on how it's doing and not finding stuff

@stefanv
Copy link
Member

stefanv commented Mar 1, 2023

Can you explain what "I hope we don't repeat what we have at SciPy" means?

In terms of dependencies: do we really need a topo sort, and when would that be useful?

@ganesh-k13
Copy link
Contributor Author

Hi sorry for the delay. I think as of now we don't have any complex dependencies and can keep it simple. As for repeating SciPy features, lets leverage pydevtool.cli if the requirement arises.

@stefanv
Copy link
Member

stefanv commented Jul 1, 2023

Some targets (test and docs specifically), now compile before executing.
I have not added this to run or any of the shell commands, since it's not as clear that you want to trigger a build before running them. E.g., you may want to try a few things on the currently built version, before building with modifications.

Either way, there's a pattern that can be extended.

I'll close this for now; feel free to open a new issue if it comes up again.

@stefanv stefanv closed this as completed Jul 1, 2023
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

3 participants