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

Add command line interface to allow scripts to be run from sasview.exe #2280

Merged
merged 36 commits into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
53dc216
Add command line interface to allow scripts to be run from sasview.exe
Oct 25, 2022
d73a757
Merge branch 'main' into ticket-2237-cli-2
pkienzle Oct 28, 2022
3a73277
Included win32 in the sasview distribution
pkienzle Oct 28, 2022
1ced3f1
Merge branch 'main' into ticket-2237-cli-2
pkienzle Oct 28, 2022
31a93d2
freeze support is only needed for the sasview.py exe script; remove i…
Oct 28, 2022
217936b
fix merge bug; sas.system.config is needed again
Oct 28, 2022
9e09ccc
sasview cli needs win32.win32console
Oct 28, 2022
39b91f4
use consistent platform query method
Oct 28, 2022
81ef0d3
Add win32 as required package
Oct 28, 2022
11eb969
Note that Qt environment variables may be needed for scripts
Oct 28, 2022
78dd442
redo command line processor so that it stops on -i/-c/-m
Oct 28, 2022
c007f6e
Allow main to return a status code.
Oct 28, 2022
0ed828d
remove no-longer-used system/env.py module
Oct 28, 2022
4ce228a
Remove nag to fix import shadowing; it's unrelated to this PR
Oct 28, 2022
8719156
Move qt environment variable setup to sas.cli
Oct 28, 2022
6160896
redo command line interface with flags for -i/-q/-V
Oct 28, 2022
cf9fa6c
pywin32 instead of win32
Oct 28, 2022
07c3efb
Updated following changes to scripting.rst
Oct 29, 2022
999bd8d
Changes to make SasView CLI help available
Oct 30, 2022
0bffa78
Add comment
Oct 31, 2022
d0d6414
Text furtling
Oct 31, 2022
ace822c
tweak help text; exit immediately after version
Oct 31, 2022
ddae208
include sasview version in the interactive startup banner
Oct 31, 2022
58691e7
Merge branch 'main' into ticket-2237-cli-2
pkienzle Nov 1, 2022
c683f64
Merge branch 'main' into ticket-2237-cli-2
Nov 8, 2022
a4e4aa2
minor tweaks from code review with no functional effect
Nov 10, 2022
1c8b2fb
Merge branch 'main' into ticket-2237-cli-2
butlerpd Dec 6, 2022
8ad3bf1
Merge branch 'main' into ticket-2237-cli-2
smk78 Jan 19, 2023
7dbdaff
Merge branch 'main' into ticket-2237-cli-2
pkienzle Feb 22, 2023
ca6864e
ticket 2237: undo merge breakage?
pkienzle Feb 23, 2023
c848247
2237: Add I/O redirection for cli to sasview.exe
pkienzle Feb 25, 2023
7e201cf
2237: Disable console redirect.
pkienzle Feb 25, 2023
f6b25a7
2237: Option to open console on startup.
Feb 28, 2023
fce99ae
2237: Change option to open console on startup.
Feb 28, 2023
efe6564
2237: Document console output option for windows.
Mar 7, 2023
de26080
Merge branch 'main' into ticket-2237-cli-2
butlerpd Mar 14, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ jobs:
python -m pip install wheel setuptools
python -m pip install -r build_tools/requirements.txt

- name: Install pywin32 (Windows)
if: ${{ startsWith(matrix.os, 'windows') }}
run: |
python -m pip install pywin32

- name: Install pyopencl (Windows)
if: ${{ startsWith(matrix.os, 'windows') }}
run: |
Expand Down
1 change: 1 addition & 0 deletions build_tools/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ importlib-resources
bumps
html2text
jsonschema
pywin32; platform_system == "Windows"
superqt
pyopengl
pyopengl_accelerate
6 changes: 5 additions & 1 deletion docs/sphinx-docs/source/user/working.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@ Working with SasView

Writing a Plugin Model <qtgui/Perspectives/Fitting/plugin>

Scripting Interface to Sasmodels <qtgui/Perspectives/Fitting/scripting>

Command Line Interpreter Syntax <qtgui/Perspectives/Fitting/cli>

Environment Variables <environment>

Model marketplace <marketplace>
Model Marketplace <marketplace>

Preferences <qtgui/MainWindow/preferences_help.rst>
10 changes: 5 additions & 5 deletions installers/sasview.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
"""

import sys


sys.dont_write_bytecode = True

from sas.qtgui.MainWindow.MainWindow import run_sasview

run_sasview()
if __name__ == "__main__":
from multiprocessing import freeze_support
freeze_support()
from sas.cli import main
sys.exit(main())
7 changes: 7 additions & 0 deletions installers/sasview.spec
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,13 @@ hiddenimports = [
'uncertainties',
]

if platform.system() == 'Windows':
# Need win32 to run sasview from the command line.
hiddenimports.extend([
'win32',
'win32.win32console',
])

a = Analysis(
['sasview.py'],
pathex=[],
Expand Down
77 changes: 29 additions & 48 deletions run.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@
import os
from os.path import abspath, dirname, realpath, join as joinpath
from contextlib import contextmanager

PLUGIN_MODEL_DIR = 'plugin_models'
from importlib import import_module

def addpath(path):
"""
Expand All @@ -35,7 +34,6 @@ def addpath(path):
os.environ['PYTHONPATH'] = PYTHONPATH
sys.path.insert(0, path)


@contextmanager
def cd(path):
"""
Expand All @@ -46,62 +44,45 @@ def cd(path):
yield
os.chdir(old_dir)

def setup_sasmodels():
"""
Prepare sasmodels for running within sasview.
"""
# Set SAS_MODELPATH so sasmodels can find our custom models

from sas.system.user import get_user_dir
plugin_dir = os.path.join(get_user_dir(), PLUGIN_MODEL_DIR)
os.environ['SAS_MODELPATH'] = plugin_dir

def prepare():
# Don't create *.pyc files
sys.dont_write_bytecode = True

# find the directories for the source and build
from distutils.util import get_platform
root = abspath(dirname(realpath(__file__)))

platform = '%s-%s' % (get_platform(), sys.version[:3])
build_path = joinpath(root, 'build', 'lib.' + platform)
# Turn numpy warnings into errors
#import numpy; numpy.seterr(all='raise')

# Notify the help menu that the Sphinx documentation is in a different
# place than it otherwise would be.
os.environ['SASVIEW_DOC_PATH'] = joinpath(build_path, "doc")

try:
import periodictable
except ImportError:
addpath(joinpath(root, '..', 'periodictable'))
# Find the directories for the source and build
root = abspath(dirname(realpath(__file__)))

try:
import bumps
except ImportError:
addpath(joinpath(root, '..', 'bumps'))
# TODO: Do we prioritize the sibling repo or the installed package?
Copy link
Contributor

Choose a reason for hiding this comment

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

When testing locally, I often have both an installed package and a repo. If I've made changes to the repo, I'd prefer to not have to pip install for every change, so I would suggest prioritizing the repo over the installed package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently it is prioritizing the installed module. We should change that...

Also, I'm using a bare except!

# TODO: Can we use sasview/run.py from a distributed sasview.exe?
# Put supporting packages on the path if they are not already available.
for sibling in ('periodictable', 'bumps', 'sasdata', 'sasmodels'):
try:
import_module(sibling)
except:
addpath(joinpath(root, '..', sibling))

# Put the source trees on the path
addpath(joinpath(root, 'src'))

# sasmodels on the path
addpath(joinpath(root, '../sasmodels/'))

# == no more C sources so no need to build project to run it ==
# Leave this snippet around in case we add a compile step later.
#from distutils.util import get_platform
#platform = '%s-%s' % (get_platform(), sys.version[:3])
#build_path = joinpath(root, 'build', 'lib.' + platform)
## Build project if the build directory does not already exist.
#if not os.path.exists(build_path):
# import subprocess
# with cd(root):
# subprocess.call((sys.executable, "setup.py", "build"), shell=False)

# Notify the help menu that the Sphinx documentation is in a different
# place than it otherwise would be.
docpath = joinpath(root, 'docs', 'sphinx-docs', '_build', 'html')
os.environ['SASVIEW_DOC_PATH'] = docpath

if __name__ == "__main__":
# Need to add absolute path before actual prepare call,
# so logging can be done during initialization process too
root = abspath(dirname(realpath(sys.argv[0])))

addpath(joinpath(root, 'src'))
prepare()

# Run the UI conversion tool when executed from script. This has to
# happen after prepare() so that sas.qtgui is on the path.
import sas.qtgui.convertUI
setup_sasmodels()

from sas.qtgui.MainWindow.MainWindow import run_sasview
run_sasview()
#logger.debug("Ending SASVIEW in debug mode.")
import sas.cli
sys.exit(sas.cli.main(logging="development"))
4 changes: 2 additions & 2 deletions src/sas/__init__.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
from sas.system.version import __version__

from sas.system import config, user
from sas.system import config

__all__ = ['config']

# TODO: fix logger-config circular dependency
# Load the config file
config.load()

184 changes: 184 additions & 0 deletions src/sas/cli.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
#
#Also see sasview\src\sas\qtgui\Perspectives\Fitting\media\cli.rst
#
"""
**SasView command line interface**

This functionality is under development. Interactive sessions do not yet
work in the Windows console.

**Usage:**

sasview [flags]
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this look like once rendered by sphinx? Would expect to have seen some more formatting on these to make a good looking output following the normal conventions for commands and options, but perhaps it looks ok as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea. I can't get the docs to build on my machine.

Copy link
Contributor

Choose a reason for hiding this comment

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

I built the docs locally and here is what I'm seeing.

sasview_cli_docs

*Run SasView. If no flag is given, or -q or -V are given, this will start
the GUI.*

sasview [flags] script [args...]
*Run a python script using the installed SasView libraries [passing
optional arguments]*

sasview [flags] -m module [args...]
*Run a SasView/Sasmodels/Bumps module as main [passing optional arguments]*

sasview [flags] -c "python statements" [args...]
*Execute python statements using the installed SasView libraries*

sasview -V
*Print sasview version and exit.*

**Flags:**

-i, --interactive. *Enter an interactive session after command/module/script.*

-o, --console. *Open a console to show command output. (Windows only)*

-q, --quiet. *Suppress startup messages on interactive console.*

Note: On Windows any console output is ignored by default. You can either
open a console to show the output with the *-o* flag or redirect output to
a file using something like *sasview ... > output.txt*.
"""
import sys

# TODO: Support dropping datafiles onto .exe?
# TODO: Maybe use the bumps cli with project file as model?

import argparse

def parse_cli(argv):
"""
Parse the command argv returning an argparse.Namespace.

* version: bool - print version
* command: str - string to exec
* module: str - module to run as main
* interactive: bool - run interactive
* args: list[str] - additional arguments, or script + args
"""
parser = argparse.ArgumentParser()
parser.add_argument("-V", "--version", action='store_true',
help="Print sasview version and exit")
parser.add_argument("-m", "--module", type=str,
help="Run module with remaining arguments sent to main")
parser.add_argument("-c", "--command", type=str,
help="Execute command")
parser.add_argument("-i", "--interactive", action='store_true',
help="Start interactive interpreter after running command")
parser.add_argument("-o", "--console", action='store_true',
help="Open console to display output (windows only)")
parser.add_argument("-q", "--quiet", action='store_true',
help="Don't print banner when entering interactive mode")
parser.add_argument("args", nargs="*",
help="script followed by args")

# Special case: abort argument processing after -m or -c.
have_trigger = False
collect_rest = False
keep = []
rest = []
for arg in argv[1:]:
# Append argument to the parser argv or collect them as extra args.
if collect_rest:
rest.append(arg)
else:
keep.append(arg)
# For an action that needs an argument (e.g., -m module) we need
# to keep the next argument for the parser, but the remaining arguments
# get collected as extra args. Trigger and collect will happen in one
# step if the trigger requires no args or if the arg was provided
# with the trigger (e.g., -mmodule)
if have_trigger:
collect_rest = True
if arg.startswith('-m') or arg.startswith('--module'):
have_trigger = True
collect_rest = arg not in ('-m', '--module')
elif arg.startswith('-c') or arg.startswith('--command'):
have_trigger = True
collect_rest = arg not in ('-c', '--command')

opts = parser.parse_args(keep)
if collect_rest:
opts.args = rest
return opts

def main(logging="production"):
Copy link
Contributor

Choose a reason for hiding this comment

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

main doesn't return but seems that it might sys.exit() from deep within. Making it so that the only way out of the function is to return an integer (that becomes the exit code) is often nicer - it's a more modular design, it's more easily tested, and the exit points are simpler to understand.

Likewise, passing the commandline args into it as a list of strings can make it easier to drive it for testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function needs to hack sys.argv to get things like bumps and ipython to work properly, so passing in args would be something of a fiction. I could write a context manager with push_argv(args): to restore on exit, but then so could the hypothetical test harness, so I'll leave it as is until needed.

from sas.system import log
from sas.system import lib
from sas.system import console

# I/O redirection for the windows console. Need to do this early so that
# output will be displayed on the console. Presently not working for
# for production (it always opens the console even if it is not needed)
# so require "sasview con ..." to open the console. Not an infamous
# "temporary fix" I hope...
if "-i" in sys.argv[1:] or "-o" in sys.argv[1:]:
console.setup_console()

# Eventually argument processing might affect logger or config, so do it first
cli = parse_cli(sys.argv)

# Setup logger and sasmodels
if logging == "production":
log.production()
elif logging == "development":
log.development()
else:
raise ValueError(f"Unknown logging mode \"{logging}\"")
lib.setup_sasmodels()
lib.setup_qt_env() # Note: does not import any gui libraries

if cli.version: # -V
import sas
print(f"SasView {sas.__version__}")
# Exit immediately after -V.
return 0

context = {'exit': sys.exit}
if cli.module: # -m module [arg...]
import runpy
# TODO: argv[0] should be the path to the module file not the dotted name
sys.argv = [cli.module, *cli.args]
context = runpy.run_module(cli.module, run_name="__main__")
elif cli.command: # -c "command"
sys.argv = ["-c", *cli.args]
exec(cli.command, context)
Copy link
Contributor

Choose a reason for hiding this comment

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

This will look like it succeeds no matter how well the command runs, unless an exception is raised. In terms of signalling success or failure, the likely code won't do that, but return true/false for example. Is there no prospect of meaningful error handling here so that it can be used in scripts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you are asking for. This behaves like python -c in the code I've tested:

$ python -c "syntax error" ; echo exit status $?
  File "<string>", line 1
    syntax error
           ^
SyntaxError: invalid syntax
exit status 1
$ python run.py -c "syntax error" ; echo exit status $?
Traceback (most recent call last):
  File "run.py", line 88, in <module>
    sys.exit(sas.cli.main(logging="development"))
  File "/Users/pkienzle/Source/sasview/src/sas/cli.py", line 131, in main
    exec(cli.command, context)
  File "<string>", line 1
    syntax error
           ^
SyntaxError: invalid syntax
exit status 1
$ python -c "import sys; sys.exit(2)" ; echo exit status $? 
exit status 2
$ python run.py -c "import sys; sys.exit(2)" ; echo exit status $?
exit status 2
$ python -c "True" ; echo exit status $?
exit status 0
$ python run.py -c "True" ; echo exit status $?    
exit status 0

Copy link
Contributor Author

@pkienzle pkienzle Feb 24, 2023

Choose a reason for hiding this comment

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

From stackoverflow here need to use "start /wait sasview" to capture the errorlevel to the windows console for windowed applications. For example:

>start /wait sasview -c "import sys; sys.exit(1)"
>echo %errorlevel%
1

elif cli.args: # script [arg...]
import runpy
sys.argv = cli.args
context = runpy.run_path(cli.args[0], run_name="__main__")
elif not cli.interactive: # no arguments so start the GUI
from sas.qtgui.MainWindow.MainWindow import run_sasview
# sys.argv is unchanged
# Maybe hand cli.quiet to run_sasview?
run_sasview()
return 0 # don't drop into the interactive interpreter

# TODO: Start interactive with ipython rather than normal python
# For ipython use:
# from IPython import start_ipython
# sys.argv = ["ipython", *args]
# sys.exit(start_ipython())
if cli.interactive:
import code
# The python banner is something like
# f"Python {sys.version} on {platform.system().lower()}"
# where sys.version contains "VERSION (HGTAG, DATE)\n[COMPILER]"
# We are replacing it with something that includes the sasview version.
if cli.quiet:
exitmsg = banner = ""
else:
import platform
import sas
# Form dotted python version number out of sys.version_info
major, minor, micro = sys.version_info[:3]
sasview_ver = f"SasView {sas.__version__}"
python_ver = f"Python {major}.{minor}.{micro}"
os_ver = platform.system()
banner = f"{sasview_ver} for {python_ver} on {os_ver}"
exitmsg = ""
code.interact(banner=banner, exitmsg=exitmsg, local=context)

return 0

if __name__ == "__main__":
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of this entry point as opposed to run.py and installers/sasview.py. This seems like a good opportunity to reduce the number of entry points into the code; ideally there would be only 1 for ease of maintenance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the real entrypoint. run.py and sasview.py prepare the environment for running.

In future when we allow pip install sasview then we can start it with python -m sas.cli. This can be handy from shell scripts and make where you replace python with $PYTHON or sys.executable in python scripts. I've used this style for a number of applications such as sphinx, nosetest and ipython. It's also handy when "borrowing" an interpreter from an environment.

Maybe we should have sas/__main__.py so that we can use python -m sas to run sasview?

Copy link
Contributor

Choose a reason for hiding this comment

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

Either this is an unneeded __main__ that no-one should call, or it is yet another __main__. sasview.py and run.py aren't calling this entry point, so this is adding another entry point. Reducing the entry points is good as you say, but this isn't doing so... yet.

I'm cautious of scope creep - this PR is not about fixing the bug that there is a multiplicity of ways of starting sasview. I'm also cautious about adding yet another way of starting sasview to the set, which then needs testing and maintenance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

run.py is not a published user interface. As a developer tool it doesn't need testing and maintenance. That is, if it is broken the developer who cares can fix it. FWIW, it has been broken since sasdata was split off and nobody has complained, so maybe I'm the only one using it. If it goes away I'll drop the equivalent into ~/bin like I currently do for sasmodels.compare.

That leaves the $ sasview.py which does:

... freeze nonsense that is required for multiprocessing on windows bundled installers ...
from sas.cli import main
sys.exit(main())

and $ python -m sas.cli which does:

if __name__ == "__main__":
    sys.exit(main())

I find that the later is more reliable since it goes through the particular python interpreter, not whatever sasview.py happens to be on the path. Indeed, the python docs give python -m pip ... as the documented interface to pip.

I'll grant that this will be more useful when we have a sasview package on pypi. Until then it requires sasview on the path, and anyone who knows how to do that can call the entry point themselves. Hint:

PYTHONPATH=/path/to/sasview/src:... python -m sas.cli ...

turns into

PYTHONPATH=/path/to/sasview/src:... python -c "import sas.cli; sys.exit(sas.cli.main())" ...

My inclination is to leave it in, but I won't object if it gets removed.

sys.exit(main())
Loading