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 ability to add custom code in documents #1259

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

XoMEX
Copy link

@XoMEX XoMEX commented Oct 11, 2024

Context

This PR adds a document setting "Custom Code".
As for the motivation, see #1258.
tl;dr: I as a Document Maintainer want to be able to define a function once in the document and use it in multiple formulas.

Proposed solution

I added a new string in the document settings. This string is inserted into the python module that is built per document.
I also added two hooks to ensure that the code is used:

  • in load_table: When the tables are read for the first time, the module is already built, but the code was not available. Once the document settings are loaded, the code is rebuilt.
  • in BulkUpdateRecord: When the document settings are updated, the whole document is invalidated to use the new code. This could be optimized, but that optimization is not easy.

While creating the change, I used a dev container. Feel free to keep or remove that.

Related issues

#1258

and "How do I add more python packages?"

If you want the import done automatically, so you don’t have to do it in formulas, currently that requires a code change to sandbox/grist/gencode.py. If you are comfortable making code changes, then the build instructions of the grist-core repository are the place to start.

This PR should also allow putting imports into the custom code.

Has this been tested?

  • 👍 yes, I added tests to the test suite
  • 💭 no, because this PR is a draft and still needs work
  • 🙅 no, because this is not relevant here
  • 🙋 no, because I need help

I did some manual testing (which I guess should be automated):

  • Setting a formula with a function that does not exist yet. Then creating that function.
  • Setting a formula with a function. Then changing that function.
  • Setting a formula with a function. Restarting Grist or just the data engine.
  • Duplicating a document with a custom function. The function is kept in the document, even when used as a template.
    Each time checking that the cells contain the expected value in the end.

I did not consider:

  • Access rights. I have no idea how they are managed in Grist.
  • Proper styling. The code input looks ok (thanks to the Ace Editor), but other than that it is missing at least a border if not more.
  • Whether validation rules are updated properly

Screenshots / Screencasts

image
image

@Chluz
Copy link

Chluz commented Oct 15, 2024

This would be a great addition - I also have complex functions I have to retype in each column. I hope the reviewers approve this and merge !

@dsagal
Copy link
Member

dsagal commented Oct 17, 2024

Thanks @XoMEX ! This is very cool. And nice to see positive feedback from others.

I'd like to summarize the wishes you've listed for this feature, and add a few.

A couple addressed by this PR:

  1. Sharing a function across multiple formula columns, e.g. Allow defining custom code/formulas to reuse #1258 (function to transform a percentage into a grade). Also requested in Allow definition of custom functions / classes at document level #386.
  2. Automating an import of a pre-installed module (e.g. instead of including in a formula import json; ... use json.dumps ..., you could include the import statement into shared code to avoid repeating it). Also requested here.

Other related wishes:
3. Sharing a suite of multiple functions. E.g. if you made a set of functions for cleaning text for different purposes, you could package it as textclean module, and import that, rather than pollute the namespace for all formulas by exposing all functions at the top level.
4. Including third-party libraries that aren't pre-installed with Grist (like pip install). That's the main question in "How do I add more python packages?"; also requested in https://community.getgrist.com/t/install-third-party-libraries/1087, and #1209.
5. Sharing a suite of useful functions across documents, and sharing with other users.

I'd be interested in having at least a rough idea how Grist might address these more ambitious wishes. Then we can decide whether those ideas would affect this PR to make sure it is both helpful on its own, and gets us closer to the bigger plan.

@XoMEX
Copy link
Author

XoMEX commented Oct 21, 2024

Thanks for considering this PR. Sometimes they PRs just get lost in a limbo state.

My thoughts for the related wishes:

  1. I see two options to do this:
    a. Offer a custom module as a separate setting. This would mean a second setting exists.
    b. Only offer a custom module as a separate setting. This means I would have to use custom.my_func() instead of my_func() (assuming the module would be named custom).
    Both are viable approaches. I can see that b might be a bit cleaner, but like the customizability of a
  2. I feel like this should be another PR as this is a good amount of work again:
  • You need to keep state which packages should be installed and which currently are. And if that is a mismatch decide what to do.
  • In docker: what happens if the container is removed and a new one is started with the same persistence data. The packages are not yet installed. Should they be installed automatically? What if an error occurs?
  • Should separate spreadsheets have different packages available (esp. versions). If yes: You need a venv per spreadsheet.
  • How to handle updating packages?
  • Should packages be imported by default? (maybe a setting per package?)
    A "good enough" solution here would be to have one global environment and have the instance admin be responsible for updates/pressing a "install now" button, but that is a decision you have to make.
  1. This is two issues:
  • across documents: That could be implemented as another module custom_server with a server wide setting. In a spreadsheet you could use custom_server.serverwide_func(). Note that exported documents then cannot be loaded on a different server that does not define the same function.
  • with other users: IMO pip support (4) would be sufficient here. Then users can either upload their packages to pypi, or just publish a git repo (pip can handle that)

@berhalak berhalak marked this pull request as draft November 6, 2024 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants