kyleam / snakemake-mode

[MIRROR] Emacs support for Snakemake
https://git.kyleam.com/snakemake-mode/about/
GNU General Public License v3.0
33 stars 8 forks source link

Support R syntax highlighting in R("""-blocks #6

Closed endrebak closed 8 years ago

endrebak commented 8 years ago

Uses purcells robust mmm-library: https://github.com/purcell/mmm-mode

Can be further updated to recognize other strings as embedded R. The template should be easy to follow.

I suggest something like """#[Rr] for the case where you need to run R-scripts using shell or write them in a variable to print/debug. But I leave it at your discretion.

Use-cases for suggestion above:

        command = """#r
        library(csaw)
        param = readParam(minq=50, restrict=c({chromosomes_as_string}))"""
        print(command)
        R(command)

or

        run_voom_script = """#R
        library(edgeR)
        normfactors = tail(read.table("{input.normalization_file}", row.names=1, header=F), 18)
        colnames(normfactors) = c("norm.factors") # need this name for voom to accept the normfactors
        df = read.table("{input.infile}")

        x = DGEList(counts=df, norm.factors=as.matrix(normfactors))

        y = voom(x)
        write.table(y$E, "{output.outfile}", sep=" ")
        """.format(**locals())

        print(run_voom_script)

        # Must craete new instance of R-interpreter to avoid clashing library requirements
        with open(output.script, "w+") as script_handle:
            script_handle.write(run_voom_script)

        shell("Rscript {output.script}")
endrebak commented 8 years ago

Sorry for this. Long time since I used git collaboratively. Guess the correct way would have been to make a new branch for each PR.

Thinking of further playing around with graphing to show the graph in emacs, but I would still use the PR-ed function to create the graph.

kyleam commented 8 years ago

Thanks, looks interesting. I've never played around with mmm-mode.

Two comments on the mmm-mode changes:

Sorry for this. Long time since I used git collaboratively. Guess the correct way would have been to make a new branch for each PR.

Thinking of further playing around with graphing to show the graph in emacs, but I would still use the PR-ed function to create the graph.

Let's go with a separate PR for the graph changes.

endrebak commented 8 years ago

Will do, but when is uncertain - doing this when I have time-consuming analyses running.

endrebak commented 8 years ago

I will open this as a separate PR if you accept the changes:

;;;;;;;;;;;;; Will be added to snakemake-mode.el

;;; Embedded language syntax-highlighting

(defun snakemake-mode-setup-mmm ()
  "Call this function in your startup-file to automatically highlight embedded R-code.

You must have the R-strings either within a R(''' ''') function call or a code block delimited with '''#r and '''."

  (if (require 'mmm-mode nil 'noerror)
      (progn (require 'mmm-mode)
             (setq mmm-global-mode 'maybe)

             (mmm-add-classes
              '((snakemake-R-call-double
                 :submode R-mode
                 :front ".*R\(\"\"\""
                 :back ".*\"\"\"\)")))

             (mmm-add-classes
              '((snakemake-R-call-regular
                 :submode R-mode
                 :front ".*R\('''"
                 :back ".*'''\)")))

             (mmm-add-classes
              '((snakemake-R-string-double
                 :submode R-mode
                 :front ".*\"\"\" * # *[rR]"
                 :back ".*\"\"\"")))

             (mmm-add-classes
              '((snakemake-R-string-regular
                 :submode R-mode
                 :front ".*''' * # *[rR]"
                 :back ".*'''")))

             (mmm-add-mode-ext-class 'snakemake-mode nil 'snakemake-R-call-double)
             (mmm-add-mode-ext-class 'snakemake-mode nil 'snakemake-R-call-regular)
             (mmm-add-mode-ext-class 'snakemake-mode nil 'snakemake-R-string-double)
             (mmm-add-mode-ext-class 'snakemake-mode nil 'snakemake-R-string-regular))
    (error "You need to install mmm-mode")))

(snakemake-mode-setup-mmm)

I guess this should be documented, but it would be strange to document this when most of the package is not documented (in any README/FAQ file). Most users probably do not read the code.

I did not make a separate mmm-snakemake-R but perhaps I should have done since people might want to add mmm support for embedded bash/fish or SQL etc.

kyleam commented 8 years ago

I will open this as a separate PR if you accept the changes:

Please open a PR. I'm for adding the change in general, but we might go through a few comment/force-push iterations.

;;; Embedded language syntax-highlighting

I'm OK with a new section, but I think adding the snakemake-mode-setup-mmm function directly under snakemake-font-lock-keywords would be fine too.

(defun snakemake-mode-setup-mmm ()
  "Call this function in your startup-file to automatically highlight embedded R-code.

Perhaps "Set up MMM mode highlighting for embedded R code."

You must have the R-strings either within a R(''' ''') function call or a code block delimited with '''#r and '''."

Please wrap this long line.

  (if (require 'mmm-mode nil 'noerror)

This code could be lifted from the if and preceded by something like

(unless (featurep 'mmm-mode)
  (user-error "You need to install mmm-mode"))

      (progn (require 'mmmm-mode)
             (setq mmm-global-mode 'maybe)

I'm not familiar with MMM mode, but, as I mentioned above, setting global configuration variable like mmm-global-mode here seems wrong. While not as convenient, I think it should just be documented that this should be set to a non-nil value.

             (mmm-add-classes
              '((snakemake-R-call-double
                 :submode R-mode
                 :front ".*R\(\"\"\""
                 :back ".*\"\"\"\)")))

             (mmm-add-classes
              '((snakemake-R-call-regular
                 :submode R-mode
                 :front ".*R\('''"
                 :back ".*'''\)")))

             (mmm-add-classes
              '((snakemake-R-string-double
                 :submode R-mode
                 :front ".*\"\"\" * # *[rR]"
                 :back ".*\"\"\"")))

Haven't tested, but doesn't this regex require a space before "#"? If so, that isn't consistent with what the docstring says.

             (mmm-add-classes
              '((snakemake-R-string-regular
                 :submode R-mode
                 :front ".*''' * # *[rR]"
                 :back ".*'''")))

             (mmm-add-mode-ext-class 'snakemake-mode nil 'snakemake-R-call-double)
             (mmm-add-mode-ext-class 'snakemake-mode nil 'snakemake-R-call-regular)
             (mmm-add-mode-ext-class 'snakemake-mode nil 'snakemake-R-string-double)
             (mmm-add-mode-ext-class 'snakemake-mode nil 'snakemake-R-string-regular))
    (error "You need to install mmm-mode")))

(snakemake-mode-setup-mmm)

This function call should be removed.

I guess this should be documented, but it would be strange to document this when most of the package is not documented (in any README/FAQ file). Most users probably do not read the code.

Right now, both snakemake-mode.el and snakemake.el in the commentary headers. snakemake-mode.el currently doesn't have any separate commands, so I think the commentary mentions everything that is supported. Aside from the new graph commands, all the snakemake.el commands should be described in the header.

I'm not against considering putting this information somewhere else, especially as more commands and functionality are added, but for the time being, perhaps it is best to document

(setq mmm-global-mode 'maybe)
(snakemake-mode-setup-mmm)

and what it provides in the the snakemake-mode.el header, extending the install and setup instructions.

I did not make a separate mmm-snakemake-R but perhaps I should have done since people might want to add mmm support for embedded bash/fish or SQL etc.

That makes sense, but I leave that up to you.

Thanks for working on this.

endrebak commented 8 years ago

You are right about the regex. Will fix the other stuff too.

Agree about the global variable. I guess I could add another unless to the setup-function that warns people that they need to set this variable to 'maybe in their startup file for automatic parsing of subregions if it is nil. Okay with you?

kyleam commented 8 years ago

I guess I could add another unless to the setup-function that warns people that they need to set this variable to 'maybe in their startup file for automatic parsing of subregions if it is nil. Okay with you?

Yes.