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.

Sign up for GitHub

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

Jump to bottom

Add import directive #46

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add import directive #46

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fdf8cf4
New directive import
meteospike Jul 10, 2024
eb5bd9e
Some internal cosmetics for import
meteospike Sep 18, 2024
2b164c2
New import regex and alias facility
meteospike Oct 9, 2024
992024e
Add check_module_name
meteospike Oct 9, 2024
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
87 changes: 75 additions & 12 deletions bin/fypp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ _SET_PARAM_REGEXP = re.compile(
_DEL_PARAM_REGEXP = re.compile(
r'^(?:[(]\s*)?[a-zA-Z_]\w*(?:\s*,\s*[a-zA-Z_]\w*)*(?:\s*[)])?$')

_IMPORT_PARAM_REGEXP = re.compile(
r'^(?:[(]\s*)?[a-zA-Z_]\w*(?:\s*,\s*[a-zA-Z_]\w*)*(?:\s*[)])?$')
Copy link
Owner

Choose a reason for hiding this comment

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

This won't allow to package.module type imports (e.g. import os.path)

Copy link
Author

Choose a reason for hiding this comment

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

Ouch ! Sorry about that !
I've made a new commit with a more suitable regex, allowing also the "as" alias. It implies some change in the import mechanism to get the actual module and not the root module in the local namespace in that case. So far, this kind of import is working fine now:

#:import pm
#:import os.path
#:import numpy as np
#:import pmore.foo as pfoo


_FOR_PARAM_REGEXP = re.compile(
r'^(?P<loopexpr>[a-zA-Z_]\w*(\s*,\s*[a-zA-Z_]\w*)*)\s+in\s+(?P<iter>.+)$')

Expand Down Expand Up @@ -349,6 +352,18 @@ class Parser:
self._log_event('del', span, name=name)


def handle_import(self, span, name):
'''Called when parser encounters an import directive.

It is a dummy method and should be overridden for actual use.

Args:
span (tuple of int): Start and end line of the directive.
name (str): Name of the python module to import.
'''
self._log_event('import', span, name=name)


def handle_if(self, span, cond):
'''Called when parser encounters an if directive.

Expand Down Expand Up @@ -652,6 +667,9 @@ class Parser:
elif directive == 'del':
self._check_param_presence(True, 'del', param, span)
self._process_del(param, span)
elif directive == 'import':
self._check_param_presence(True, 'import', param, span)
self._process_import(param, span)
elif directive == 'for':
self._check_param_presence(True, 'for', param, span)
self._process_for(param, span)
Expand Down Expand Up @@ -765,6 +783,14 @@ class Parser:
self.handle_del(span, param)


def _process_import(self, param, span):
match = _IMPORT_PARAM_REGEXP.match(param)
if not match:
msg = "invalid module name specification '{0}'".format(param)
raise FyppFatalError(msg, self._curfile, span)
self.handle_import(span, param)


def _process_for(self, param, span):
match = _FOR_PARAM_REGEXP.match(param)
if not match:
Expand Down Expand Up @@ -1196,6 +1222,16 @@ class Builder:
self._curnode.append(('del', self._curfile, span, name))


def handle_import(self, span, name):
'''Should be called to signalize an import directive.

Args:
span (tuple of int): Start and end line of the directive.
name (str): Name of the module to import.
'''
self._curnode.append(('import', self._curfile, span, name))


def handle_eval(self, span, expr):
'''Should be called to signalize an eval directive.

Expand Down Expand Up @@ -1424,6 +1460,8 @@ class Renderer:
output.append(result)
elif cmd == 'del':
self._delete_variable(*node[1:4])
elif cmd == 'import':
self._load_module(*node[1:4])
elif cmd == 'for':
out, ieval, peval = self._get_iterated_content(*node[1:6])
eval_inds += _shiftinds(ieval, len(output))
Expand Down Expand Up @@ -1687,6 +1725,20 @@ class Renderer:
return result


def _load_module(self, fname, span, name):
result = ''
try:
self._evaluator.loadmodules(name)
except Exception as exc:
msg = "exception occurred when importing module(s) '{0}'"\
.format(name)
raise FyppFatalError(msg, fname, span) from exc
multiline = (span[0] != span[1])
if self._linenums and not self._diverted and multiline:
result = self._linenumdir(span[1], fname)
return result


def _add_global(self, fname, span, name):
result = ''
try:
Expand Down Expand Up @@ -2059,6 +2111,18 @@ class Evaluator:
raise FyppFatalError(msg)


def loadmodules(self, name):
'''Load modules in current space name.

Args:
name (str): Name(s) of the module(s) to load.
'''
modnames = self._get_variable_names(name)
for modname in modnames:
self._check_variable_name(modname)
Copy link
Owner

Choose a reason for hiding this comment

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

We should have a different check, as module names might differ from variable names (e.g. "." in the name)

Copy link
Author

Choose a reason for hiding this comment

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

Ok, the check is on prefix or reserved names, but a new commit add a _check_module_name anyway, called with module name or alias name if defined.

self.import_module(modname)


def addglobal(self, name):
'''Define a given entity as global.

Expand Down Expand Up @@ -2374,6 +2438,7 @@ class Processor:
self._parser.handle_enddef = self._builder.handle_enddef
self._parser.handle_set = self._builder.handle_set
self._parser.handle_del = self._builder.handle_del
self._parser.handle_import = self._builder.handle_import
self._parser.handle_global = self._builder.handle_global
self._parser.handle_for = self._builder.handle_for
self._parser.handle_endfor = self._builder.handle_endfor
Expand Down Expand Up @@ -2496,18 +2561,22 @@ class Fypp:
def __init__(self, options=None, evaluator_factory=Evaluator,
parser_factory=Parser, builder_factory=Builder,
renderer_factory=Renderer):
syspath = self._get_syspath_without_scriptdir()
self._adjust_syspath(syspath)
if options is None:
options = FyppOptions()
syspath = self._get_syspath_without_scriptdir()
lookuppath = []
if options.moduledirs is not None:
lookuppath += [os.path.abspath(moddir) for moddir in options.moduledirs]
lookuppath.append(os.path.abspath('.'))
lookuppath += syspath
self._adjust_syspath(lookuppath)
Comment on lines +2583 to +2589
Copy link
Owner

Choose a reason for hiding this comment

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

This is somewhat problematic, because it would change the sys.path permanently for the rest of the code. Originally, the syspath manipulation was in effect during the import of the user specified modules only. Is there any reason, why you want to make the changes permanent instead?

if inspect.signature(evaluator_factory) == inspect.signature(Evaluator):
evaluator = evaluator_factory()
else:
raise FyppFatalError('evaluator_factory has incorrect signature')
self._encoding = options.encoding
if options.modules:
self._import_modules(options.modules, evaluator, syspath,
options.moduledirs)
self._import_modules(options.modules, evaluator)
if options.defines:
self._apply_definitions(options.defines, evaluator)
if inspect.signature(parser_factory) == inspect.signature(Parser):
Expand Down Expand Up @@ -2603,16 +2672,10 @@ class Fypp:
evaluator.define(name, value)


def _import_modules(self, modules, evaluator, syspath, moduledirs):
lookuppath = []
if moduledirs is not None:
lookuppath += [os.path.abspath(moddir) for moddir in moduledirs]
lookuppath.append(os.path.abspath('.'))
lookuppath += syspath
self._adjust_syspath(lookuppath)
@staticmethod
def _import_modules(modules, evaluator):
for module in modules:
evaluator.import_module(module)
self._adjust_syspath(syspath)


@staticmethod
Expand Down