ENH: Developer CLI for SciPy by sayantikabanik · Pull Request #15959 · scipy/scipy

sayantikabanik · 2022-04-08T15:34:32Z

Reference issue

Part of RFC: unify all developer CLI tools #15489
POC details Prototype a better developer CLI for SciPy (& similar projects) rgommers/scipy#133

What does this implement/fix?

Implements a developer CLI using doit, click along with a GUI implemented using rich-click.

Handy commands

Enabling the GUI: python do.py
Listing the args/options for a task: python do.py <task_name> --help

Additional information

Current list of implemented tasks:

Build & testing tasks
- build (build & install package on path)
- test (Run tests along with options to run tests for a given submodule)
Static checker tasks
- pep8 ( Perform pep8 check with flake8)
- mypy ( Run mypy on the codebase)
Environments
- shell (Start Unix shell with PYTHONPATH set)
- python (Start a Python shell with PYTHONPATH set)
- ipython (Start IPython shell with PYTHONPATH set )
Documentation tasks
- doc (Build documentation)
- refguide-check (Run refguide check)
Release tasks
- notes (Release notes and log generation)
- authors (Task to generate list the authors who contributed within a given revision interval)
Benchmarking tasks
- bench & bench compare

GUI look

cc @rgommers @schettino72

…opts.

…ion of commnads/tasks.

…ough method "meta()". Add command "doc".

…tions into sections.

…to cli-doit

…ipy#15896)

Fix bench handling of CWD. Do not rely on global value of installed path.

tupui · 2022-04-08T16:00:13Z

The GUI looks awesome! Great work 👏

rgommers

Thanks @sayantikabanik and @schettino72 for all the hard work to put together this developer interface - I think it will be a major upgrade to the experience of working on SciPy!

@scipy/maintainers this isn't 100% final but we've been using and tweaking it for a little while, and it's ready for merging now imho. Smaller things can be added/fixed in follow-ups. I'll add here the current plan for this CLI, and what I think are the main things to look at (aside from just trying the workflows you care about and playing around) for review.

The plan:

Merge this PR in its current state (modulo review comments), with experimental status - so no docs yet, and reserve the freedom to make some changes over the next weeks.
There's still a part of this in do.py (clearly marked) that @schettino72 wants to move into either doit or a separate package. That will simplify maintenance of this file, and make it reusable by other projects.
Once we're happy with this CLI, remove dev.py and rename do.py to dev.py (better name, and do is a bash builtin).
Update contributor docs for this (note that a lot of docs are in --help for each command, so we need fewer html docs).
After 1.9.0, we can remove numpy.distutils build infra, and hence also runtests.py. At that point we have a single CLI again, this one.

To review:

The plan above
The added dependencies. They're not too heavy, but there are a few new ones. I think the screenshot in the PR description already shows that that's worth it.
The complexity of the logic, for build, test and bench tasks in particular. I think it's an improvement over the massive if-elif-else block in dev.py/runtests.py. But maybe there's room for further improvement.

tupui

I agree with the plan, we should not spend time on nitpicks there. Provided the added commands are working, I am fine with getting it in to iterate faster. If we start using this in CI, we will see rapidly what is working and what not.

On rich itself, I am neutral. It does look nicer, but it's not essential and rich is still moving very fast. But now that pip started to use it, it might be a sign of confidence/stability/guarantee.

tupui · 2022-04-08T16:06:04Z

+  - click
+  - pip:


We can already remove this I think since the release on conda is in progress.

Updated, though for arm64 it failed with PackagesNotFoundError:. It's not yet available under conda-forge/osx-arm64 or conda-forge/noarch

Opened an issue for that at conda-forge/doit-feedstock#16

Looks like we won't get that arm64 package, so let's revert back to pip until there's a new doit release which is noarch.

tupui · 2022-04-08T16:09:11Z

+
+BUG:
+- [ ] python dev.py -t scipy.optimize.tests.test_minimize_constrained.py::TestTrustRegionConstr::test_args
+      unknown marker "slow". seems conftest is not being loaded


I thought this was solved with latest pytest. They had a regression on 7.1 and 7.1.1 should fix it.

It still fails for me on pytest 7.1.1
Note the command line above reproduces an error on existing dev.py, not on the code in this PR.

Failed for me as well using pytest 7.1.1

pytest.PytestUnknownMarkWarning: Unknown pytest.mark.slow

@sayantikabanik feel free to look into this. I am busy with other stuff...

I made an attempt to fix the above (using custom marks), though wasn't able to arrive at the expected outcome.

🤦 The problem is the example is using python module notation (separated by dot), but also includes .py.
A pytest bug is responsible for the misleading error message...

WRONG:

python dev.py -t scipy.optimize.tests.test_minimize_constrained.py::TestTrustRegionConstr::test_args

CORRECT:

python dev.py -t scipy.optimize.tests.test_minimize_constrained::TestTrustRegionConstr::test_args python dev.py -t scipy/optimize/tests/test_minimize_constrained.py::TestTrustRegionConstr::test_args

rgommers · 2022-04-08T16:48:33Z

+# TODO:
+First milestone replace dev.py
+
+- [ ] doc: it would make sense to expose MAKE targets as sub-commands or options


Note that I plan to seriously cull what's in that Makefile. We really only need html, show, upload, dist. That's for another time though, for now what's in the CLI is enough.

schettino72 · 2022-04-08T17:46:56Z

Thanks @rgommers and @sayantikabanik for support and collaboration.

I would just like to add to the plan (at least from my side and if you guys agree).
This PR has improvements for the CLI itself...

But now since commands tasks are based on doit, there is room for improvements on individual tasks.
Mostly speed up by taking advantage of up-to-date checks and incremental execution.

rgommers · 2022-04-08T18:46:57Z

But now since commands tasks are based on doit, there is room for improvements on individual tasks.
Mostly speed up by taking advantage of up-to-date checks and incremental execution.

This would be interesting in principle, anything where we can improve the lag on the development cycle will be quite helpful. The question is which commands though. Right now, every tool has its own cache, at least these ones:

Meson (build)
Sphinx (doc build)
ASV (benchmarks, and build envs for --compare)
Mypy (type annotations, a bit flaky IIRC)

The Meson cache is reliable and fast, and doit doesn't have enough info to skip invoking Meson. For ASV there may be an opportunity to skip the "environment up to date" checks, not sure. Sphinx is glacially slow, but it's all a single step, so not sure what doit can do about that. Mypy the same I think.

Perhaps the bigger gain here would be for composite commands, like "run through the whole release workflow for tag v1.8.0". Thinking about that some more, it's the composability of tasks more than skipping tasks that will be helpful. The dev.py style if-else logic make it quite hard to extend. Now we can more easily define new tasks that depend on other tasks. Those multi-task jobs where doit is in control are then in turn much easier to ensure "task is up-to-date and hence can be skipped" is feasible, because there's no environment activation, change in build configuration, etc. that can mess things up in between two tasks.

rgommers · 2022-04-12T15:49:24Z

Hi all, I plan to merge this by Friday unless there's blocking concerns. That will help get people to use it and find possible parts that aren't working smoothly just yet. We'll keep dev.py around until we're sure this is all good to go.

grlee77 · 2022-04-14T15:45:40Z

This looks great! I tried the test, doc and pep8 commands and they ran as expected. The only issue I noticed is that python do.py pep8 does result in a lot of flake8 warnings about the new do.py itself.

schettino72 · 2022-04-14T15:51:25Z

@grlee77 I have just started to work on fixing flake8 issues. I should be able to push an update today.

schettino72 · 2022-04-14T21:42:15Z

I fixed the pep8 issues in do.py.

Minor issue: I guess we should take this opportunity to rename the command pep8.
pep8 package was renamed to pycodestyle...
And tool used is actually flake8 which uses executes pycodestyle and pyflakes internally.

rgommers · 2022-04-15T14:42:22Z

Minor issue: I guess we should take this opportunity to rename the command pep8.

Yes you're right, I'd suggest we call it lint because what it does is linting in the same way as we do in CI (where it's also called lint). It actually does two separate things:

Run flake8 over the whole code base
Check for PEP 8 compliance on the diff between the branch and main by running the tools/lint_diff.py script.

I'll push a change.

Wrench emoji means "depends on build", so invent a new emoji for "runs fast, no build required". [ci skip]

rgommers · 2022-04-15T14:52:28Z

I added a new emoji for "doesn't depend on the build task" to make sure things line up.

rgommers

Hi all, I plan to merge this by Friday unless there's blocking concerns. That will help get people to use it and find possible parts that aren't working smoothly just yet. We'll keep dev.py around until we're sure this is all good to go.

It is now Friday, and this has been tested by a fair number of people. So let's merge this as is. A huge thank you to @schettino72 and @sayantikabanik for doing the heavy lifting. And of course to everyone else who contributed ideas/testing/review.

There are a few follow-ups listed in the file itself. Others I'm aware of:

clean up docs Makefile
add support for act in do.py (@sayantikabanik has changes ready for that)
once the doit conda package is installable as noarch, update the environment yml files
look at new opportunities provided by the new structure (caching w doit, more complex workflows, ...?)

And one that just occurred to me and may make sense:

make the --help output paging, because it's too long to fit on the average terminal.

sayantikabanik · 2022-04-20T09:28:58Z

add support for act in do.py (@sayantikabanik has changes ready for that)

Currently facing an issue which is also reported under #16009
Post the fix, I can run a few tests.

[macOS tests (meson)/Meson build] Skipping job 'Meson build' due to 'github.repository == 'scipy/scipy' || github.repository == '''

AnirudhDagar · 2022-05-04T05:48:45Z

When there is any kind of error, it is not logged on the terminal. Instead, the script just exits without telling anything.

How to reproduce

Just add any random erroneous line to do.py file task.

scipy/do.py

Lines 545 to 551 in 9137ef1

    
               @classmethod 
        
               def build_project(cls, dirs, args, env): 
        
                   """ 
        
                   Build a dev version of the project. 
        
                   """ 
        
                   cmd = ["ninja", "-C", str(dirs.build)] 
        
                   if args.parallel > 1:

Here I'm adding the following before line 550.

     @classmethod 
     def build_project(cls, dirs, args, env): 
         """ 
         Build a dev version of the project. 
         """ 
         import thisdoesnotexist  #obviously this should error out
         cmd = ["ninja", "-C", str(dirs.build)] 
         if args.parallel > 1:

But when I run python do.py build, the script just runs and exits at this line without any error or traceback output.
Is this issue already known since do.py is under development atm? Asking here quickly, otherwise, I'll open up an issue.

cc @sayantikabanik @schettino72 @rgommers

AnirudhDagar · 2022-05-04T05:50:53Z

nevermind, just found this comment rgommers#133 (comment)

Apologies for the noise.

schettino72 and others added 30 commits March 17, 2022 02:41

BLD: doit tasks + click CLI. No support for task parameters.

c71785a

BLD: click CLI with Group. doit API to run specified task with param/…

8b052ee

…opts.

BLD: [WIP] dev CLI with click + doit. Use class based API for definit…

21d27dd

…ion of commnads/tasks.

BLD: doit+click, fixes for "test" command.

a0b7e09

BLD: doit+click, add support for Task creating doit task metadata thr…

820b662

…ough method "meta()". Add command "doc".

BLD: doit+click, add "refcheck-guide".

8de76c0

DEV: do.py uses rich-click for help color output and split command/op…

5de2596

…tions into sections.

DEV: do.py, add basic doc on creating cmds/tasks.

310456e

DEV: release notes and log generation task

1bd6721

Merge branch 'scipy:main' into cli-doit

222a41e

DEV: author list gerneration task

61edc83

Merge branch 'cli-doit' of https://github.com/sayantikabanik/scipy in…

77ab8d3

…to cli-doit

DEV: markdown changes incorporated

0db9d0f

DEV: reverting to emoji shortcodes (based on latest rich-click release)

c3148a3

DEV: TODOs updated

842a286

DEV: Basic sketeton added for benchmarking task [WIP]

ea6da9b

DEV: Removed lcov_html from TODO list, currently broken (details : sc…

9b344cb

…ipy#15896)

DEV: options identified and class modified

4d86adb

DEV: Module path and version fix for bench task

939233d

DEV: Redundant print statement removed

b912331

DEV: short option and TODOs updated for bench task

fa50de1

DEV: Example added for bench task

36c7b5b

DEV: bench task modified, docstrings updated

d5f73f0

DEV: bench-compare option updated

fb87c73

DEV: restructuring options

7c630e2

DEV: do.py, implement --no-build and handle positional arguments.

16fadd8

DEV: do.py, remove dependency on dev.py and runtests.py.

9233c90

Fix bench handling of CWD. Do not rely on global value of installed path.

DEV: do.py, replace os.path with pathlib. Remove global ROOT_DIR.

17eb029

DEV: do.py, refactor (DRY) env commands. fix bench PATH.

f42e72d

DEV: Requirements section added

b459a67

rgommers added DX Everything related to making the experience of working on SciPy more pleasant enhancement A new feature or improvement labels Apr 8, 2022

rgommers reviewed Apr 8, 2022

View reviewed changes

tupui reviewed Apr 8, 2022

View reviewed changes

rgommers reviewed Apr 8, 2022

View reviewed changes

doit channel updated

2365294

sayantikabanik added 3 commits April 13, 2022 09:33

[ci skip] reverting to pypi doit package

1a0a96d

[ci skip] gcov option updated

f06b18b

[ci skip] macOS meson CI job updated to use do.py

b1c12bf

schettino72 added 2 commits April 15, 2022 03:43

DEV: do.py, fix flake8 issues.

348da04

DEV: do.py, set doit reporter when executed through click CLI.

5af8ac4

rgommers added this to the 1.9.0 milestone Apr 15, 2022

DOC: rename pep8 task to lint (same as in CI), and add emoji

b9e9367

Wrench emoji means "depends on build", so invent a new emoji for "runs fast, no build required". [ci skip]

rgommers approved these changes Apr 15, 2022

View reviewed changes

rgommers merged commit b5ffe94 into scipy:main Apr 15, 2022

AnirudhDagar mentioned this pull request Apr 16, 2022

CI: move Azure jobs to GitHub Actions #15814

Closed

thomasjpfan mentioned this pull request Apr 17, 2022

MAINT: Adds doit.db.db to gitignore #15999

Merged

tylerjereddy mentioned this pull request Oct 3, 2022

Automated formatting tool for development kokkos/pykokkos#103

Open

Uh oh!

Conversation

sayantikabanik commented Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference issue

What does this implement/fix?

Handy commands

Additional information

GUI look

Uh oh!

tupui commented Apr 8, 2022

Uh oh!

rgommers left a comment

Choose a reason for hiding this comment

Uh oh!

tupui left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tupui Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tupui Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sayantikabanik Apr 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

schettino72 commented Apr 8, 2022

Uh oh!

rgommers commented Apr 8, 2022

Uh oh!

rgommers commented Apr 12, 2022

Uh oh!

grlee77 commented Apr 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

schettino72 commented Apr 14, 2022

Uh oh!

schettino72 commented Apr 14, 2022

Uh oh!

rgommers commented Apr 15, 2022

Uh oh!

rgommers commented Apr 15, 2022

Uh oh!

rgommers left a comment

Choose a reason for hiding this comment

Uh oh!

sayantikabanik commented Apr 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AnirudhDagar commented May 4, 2022

How to reproduce

Uh oh!

AnirudhDagar commented May 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

sayantikabanik commented Apr 8, 2022 •

edited

Loading

tupui left a comment •

edited

Loading

tupui Apr 8, 2022 •

edited

Loading

tupui Apr 8, 2022 •

edited

Loading

sayantikabanik Apr 14, 2022 •

edited

Loading

grlee77 commented Apr 14, 2022 •

edited

Loading

sayantikabanik commented Apr 20, 2022 •

edited

Loading