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

feat: demo script to proc and replace files with shell commands #464

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

Conversation

ankostis
Copy link

@ankostis ankostis commented May 8, 2023

Adapted from lint-histoty.py demo, but command's output can be a different file which is added in the history,
and optionally replaces completely the source file.

For context, I needed this one to post-process my jupyterlab notebooks (json-files) and convert them into py:percent regular python files with jupytext so as git-diff to work decently.

@ankostis
Copy link
Author

Is there something i can do to approve CI workflow for this PR?

adapted  from lint demo,  but commands output can be a different file.

Signed-off-by: Kostis Anagnostopoulos <[email protected]>
@huyz
Copy link

huyz commented Apr 24, 2024

Could there be a way to add this functionality to the original lint-history with a flag?

@newren
Copy link
Owner

newren commented Jul 7, 2024

Sorry for the long delay.

This PR is very interesting. I think @huyz's comment is a good one, though: this would give us a very heavy amount of code duplication (and even a copy of a now-obsolete large comment block) between lint-history and your script. While you make several stylistic changes throughout, those stylistic changes are equally applicable to lint-history and we really should probably either have both take the new style or both take the old style (to be determined individually for each stylistic change). From a high level point of view, though, namely from the user's perspective, your script really is just lint-history with two additional parameters (--outfpath && --drop-src). As such, I think it'd be better to structure it as an extension to that file.

Also, the commit message looks like it got mangled.

But, at an even higher level, why is this script needed? Users could already run e.g.

lint-history --relevant 'return filename.lower().endswith(b".ipynb")' -- jupytext --to 'py:percent'
git filter-repo --filename-callback 'return filename[:-6] + b".py" if filename.lower().endswith(b".ipynb") else filename'

In other words, modify the contents by calling lint-history, and then rename in a separate step (using git filter-repo).

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.

4 participants