Merge pull request #148 from krassowski/fix/r-magics

Enhanced argument parsing in rpy2 magics

Author: krassowski

CHANGELOG.md

@@ -9,6 +9,7 @@
 ## `@krassowski/jupyterlab-lsp 0.7.0-rc.0`
 
 - features
+
   - reduced space taken up by the statusbar indicator
   - implemented statusbar popover with connections statuses
   - generates types for server data responses from JSON schema (
@@ -32,6 +33,12 @@
   - bash LSP now also covers `%%bash` magic cell in addition to `%%sh` (
     [#144](https://github.com/krassowski/jupyterlab-lsp/pull/144)
     )
+  - rpy2 magics received enhanced support for argument parsing
+    in both parent Python document (re-written overrides) and
+    exctracted R documents (improved foreign code extractor) (
+    [#148](https://github.com/krassowski/jupyterlab-lsp/pull/148)
+    )
+
 - bugfixes
   - diagnostics in foreign documents are now correctly updated (
     [133fd3d](https://github.com/krassowski/jupyterlab-lsp/pull/129/commits/133fd3d71401c7e5affc0a8637ee157de65bef62)

packages/jupyterlab-lsp/src/extractors/defaults.spec.ts

@@ -0,0 +1,106 @@
+import { IVirtualDocumentBlock, VirtualDocument } from '../virtual/document';
+import { expect } from 'chai';
+import { foreign_code_extractors } from './defaults';
+import { CodeEditor } from '@jupyterlab/codeeditor';
+
+function wrap_in_python_lines(line: string) {
+  return 'print("some code before")\n' + line + '\n' + 'print("and after")\n';
+}
+
+describe('Default extractors', () => {
+  let document: VirtualDocument;
+
+  function extract(code: string) {
+    return document.extract_foreign_code(code, null, {
+      line: 0,
+      column: 0
+    });
+  }
+
+  function get_the_only_virtual(
+    foreign_document_map: Map<CodeEditor.IRange, IVirtualDocumentBlock>
+  ) {
+    expect(foreign_document_map.size).to.equal(1);
+
+    let { virtual_document } = foreign_document_map.get(
+      foreign_document_map.keys().next().value
+    );
+
+    return virtual_document;
+  }
+
+  beforeEach(() => {
+    document = new VirtualDocument(
+      'python',
+      'test.ipynb',
+      {},
+      foreign_code_extractors,
+      false,
+      'py',
+      false
+    );
+  });
+
+  afterEach(() => {
+    document.clear();
+  });
+
+  describe('%R line magic', () => {
+    it('extracts simple commands', () => {
+      let code = wrap_in_python_lines('%R ggplot()');
+      let { cell_code_kept, foreign_document_map } = extract(code);
+
+      // should not be removed, but left for the static analysis (using magic overrides)
+      expect(cell_code_kept).to.equal(code);
+      let r_document = get_the_only_virtual(foreign_document_map);
+      expect(r_document.language).to.equal('r');
+      expect(r_document.value).to.equal('ggplot()\n');
+    });
+
+    it('parses input (into a dummy data frame)', () => {
+      let code = wrap_in_python_lines('%R -i df ggplot(df)');
+      let { foreign_document_map } = extract(code);
+
+      let r_document = get_the_only_virtual(foreign_document_map);
+      expect(r_document.language).to.equal('r');
+      expect(r_document.value).to.equal('df <- data.frame()\nggplot(df)\n');
+    });
+
+    it('parses multiple inputs (into dummy data frames)', () => {
+      let code = wrap_in_python_lines('%R -i df -i x ggplot(df)');
+      let r_document = get_the_only_virtual(extract(code).foreign_document_map);
+      expect(r_document.value).to.equal(
+        'df <- data.frame()\n' + 'x <- data.frame()\n' + 'ggplot(df)\n'
+      );
+    });
+
+    it('parses inputs ignoring other arguments', () => {
+      let code = wrap_in_python_lines('%R -i df --width 300 -o x ggplot(df)');
+      let r_document = get_the_only_virtual(extract(code).foreign_document_map);
+      expect(r_document.value).to.equal(
+        'df <- data.frame()\n' + 'ggplot(df)\n'
+      );
+    });
+  });
+
+  describe('%%R cell magic', () => {
+    it('extracts simple commands', () => {
+      let code = '%%R\nggplot()';
+      let { cell_code_kept, foreign_document_map } = extract(code);
+
+      expect(cell_code_kept).to.equal(code);
+      let r_document = get_the_only_virtual(foreign_document_map);
+      expect(r_document.language).to.equal('r');
+      expect(r_document.value).to.equal('ggplot()\n');
+    });
+  });
+
+  it('parses input (into a dummy data frame)', () => {
+    let code = '%%R -i df\nggplot(df)';
+    let { foreign_document_map } = extract(code);
+
+    let r_document = get_the_only_virtual(foreign_document_map);
+    expect(r_document.language).to.equal('r');
+    expect(r_document.value).to.equal('df <- data.frame()\nggplot(df)\n');
+  });
+});

packages/jupyterlab-lsp/src/extractors/defaults.ts

@@ -1,5 +1,20 @@
 import { IForeignCodeExtractorsRegistry } from './types';
 import { RegExpForeignCodeExtractor } from './regexp';
+import {
+  extract_r_args,
+  rpy2_args_pattern,
+  RPY2_MAX_ARGS
+} from '../magics/rpy2';
+
+function rpy2_replacer(match: string, ...args: string[]) {
+  let r = extract_r_args(args, -3);
+  // define dummy input variables using empty data frames
+  let inputs = r.inputs.map(i => i + ' <- data.frame()').join('\n');
+  if (inputs !== '') {
+    inputs += '\n';
+  }
+  return `${inputs}${r.rest}`;
+}
 
 // TODO: make the regex code extractors configurable
 export let foreign_code_extractors: IForeignCodeExtractorsRegistry = {
@@ -10,15 +25,15 @@ export let foreign_code_extractors: IForeignCodeExtractorsRegistry = {
     //
     new RegExpForeignCodeExtractor({
       language: 'r',
-      pattern: '^%%R( .*?)?\n([^]*)',
-      extract_to_foreign: '$2',
+      pattern: '^%%R' + rpy2_args_pattern(RPY2_MAX_ARGS) + '\n([^]*)',
+      extract_to_foreign: rpy2_replacer,
       is_standalone: false,
       file_extension: 'R'
     }),
     new RegExpForeignCodeExtractor({
       language: 'r',
-      pattern: '(^|\n)%R (.*)\n?',
-      extract_to_foreign: '$2',
+      pattern: '(?:^|\n)%R' + rpy2_args_pattern(RPY2_MAX_ARGS) + ' (.*)\n?',
+      extract_to_foreign: rpy2_replacer,
       is_standalone: false,
       file_extension: 'R'
     }),

packages/jupyterlab-lsp/src/extractors/regexp.ts

@@ -1,5 +1,6 @@
 import { IExtractedCode, IForeignCodeExtractor } from './types';
 import { position_at_offset } from '../positioning';
+import { replacer } from '../magics/overrides';
 
 export class RegExpForeignCodeExtractor implements IForeignCodeExtractor {
   options: RegExpForeignCodeExtractor.IOptions;
@@ -39,6 +40,7 @@ export class RegExpForeignCodeExtractor implements IForeignCodeExtractor {
       let matched_string = match[0];
       let foreign_code_fragment = matched_string.replace(
         this.expression,
+        // @ts-ignore
         this.options.extract_to_foreign
       );
 
@@ -110,7 +112,7 @@ namespace RegExpForeignCodeExtractor {
      * for the use in virtual document of the foreign language.
      * For the R example this should be '$3'
      */
-    extract_to_foreign: string;
+    extract_to_foreign: string | replacer;
     /**
      * String boolean if everything (true, default) or nothing (false) should be kept in the host document.
      *

packages/jupyterlab-lsp/src/magics/defaults.ts

@@ -1,6 +1,8 @@
 import { IOverridesRegistry } from './overrides';
 import {
   parse_r_args,
+  rpy2_args_pattern,
+  RPY2_MAX_ARGS,
   rpy2_reverse_pattern,
   rpy2_reverse_replacement
 } from './rpy2';
@@ -46,7 +48,7 @@ export const language_specific_overrides: IOverridesRegistry = {
       },
       {
         // support up to 10 arguments
-        pattern: '%R' + '(?: -(\\S+) (\\S+))?'.repeat(10) + '(.*)(\n)?',
+        pattern: '%R' + rpy2_args_pattern(RPY2_MAX_ARGS) + '(.*)(\n)?',
         replacement: (match, ...args) => {
           let r = parse_r_args(args, -4);
           return `${r.outputs}rpy2.ipython.rmagic.RMagics.R("${r.content}", "${r.others}"${r.inputs})`;
@@ -81,7 +83,7 @@ export const language_specific_overrides: IOverridesRegistry = {
       // rpy2 extension R magic - this should likely go as an example to the user documentation, rather than being a default
       //   only handles simple one input - one output case
       {
-        pattern: '^%%R' + '(?: -(\\S) (\\S+))?'.repeat(10) + '(\n)?([\\s\\S]*)',
+        pattern: '^%%R' + rpy2_args_pattern(RPY2_MAX_ARGS) + '(\n)?([\\s\\S]*)',
         replacement: (match, ...args) => {
           let r = parse_r_args(args, -3);
           return `${r.outputs}rpy2.ipython.rmagic.RMagics.R("""${r.content}""", "${r.others}"${r.inputs})`;

packages/jupyterlab-lsp/src/magics/rpy2.spec.ts

@@ -32,7 +32,7 @@ describe('rpy2 IPython overrides', () => {
     let line_magics_map = new LineMagicsMap(
       language_specific_overrides['python'].line_magics
     );
-    it('inputs and outputs', () => {
+    it('works with the short form arguments, inputs and outputs', () => {
       let line = '%R -i x';
       let override = line_magics_map.override_for(line);
       expect(override).to.equal('rpy2.ipython.rmagic.RMagics.R("", "", x)');
@@ -75,5 +75,22 @@ describe('rpy2 IPython overrides', () => {
       reverse = line_magics_map.reverse.override_for(override);
       expect(reverse).to.equal(line);
     });
+
+    it('works with the long form arguments', () => {
+      let line = '%R --input x';
+      let override = line_magics_map.override_for(line);
+      expect(override).to.equal('rpy2.ipython.rmagic.RMagics.R("", "", x)');
+      let reverse = line_magics_map.reverse.override_for(override);
+      // TODO: make this preserve the long form
+      expect(reverse).to.equal('%R -i x');
+
+      line = '%R --width 800 --height 400 command()';
+      override = line_magics_map.override_for(line);
+      expect(override).to.equal(
+        'rpy2.ipython.rmagic.RMagics.R(" command()", "--width 800 --height 400")'
+      );
+      reverse = line_magics_map.reverse.override_for(override);
+      expect(reverse).to.equal(line);
+    });
   });
 });

packages/jupyterlab-lsp/src/magics/rpy2.ts

@@ -1,4 +1,6 @@
-export function parse_r_args(args: string[], content_position: number) {
+export const RPY2_MAX_ARGS = 10;
+
+export function extract_r_args(args: string[], content_position: number) {
   let inputs = [];
   let outputs = [];
   let others = [];
@@ -7,15 +9,27 @@ export function parse_r_args(args: string[], content_position: number) {
     let variable = args[i + 1];
     if (typeof arg === 'undefined') {
       break;
-    } else if (arg === 'i') {
+    } else if (arg === 'i' || arg === '-input') {
       inputs.push(variable);
-    } else if (arg === 'o') {
+    } else if (arg === 'o' || arg === '-output') {
       outputs.push(variable);
     } else {
       others.push('-' + arg + ' ' + variable);
     }
   }
-  let rest = args.slice(content_position, content_position + 1);
+  return {
+    inputs: inputs,
+    outputs: outputs,
+    rest: args.slice(content_position, content_position + 1),
+    others: others
+  };
+}
+
+export function parse_r_args(args: string[], content_position: number) {
+  let { inputs, outputs, rest, others } = extract_r_args(
+    args,
+    content_position
+  );
   let input_variables = inputs.join(', ');
   if (input_variables) {
     input_variables = ', ' + input_variables;
@@ -81,3 +95,7 @@ export function rpy2_reverse_replacement(match: string, ...args: string[]) {
     contents: contents
   };
 }
+
+export function rpy2_args_pattern(max_n: number) {
+  return '(?: -(\\S+) (\\S+))?'.repeat(max_n);
+}

packages/jupyterlab-lsp/src/virtual/document.ts

@@ -29,7 +29,7 @@ interface IVirtualLine {
   editor: CodeMirror.Editor;
 }
 
-interface IVirtualDocumentBlock {
+export interface IVirtualDocumentBlock {
   /**
    * Line corresponding to the block in the entire foreign document
    */