Tex selective cleanup

[digmanwaves]

Overview: What does this pull request change?

In the current version, the call to delete_nonsvg_files (after a TeX run) deletes also files that are not related to the current TeX job. In the new version, an argument 'jobname' was added to delete_nonsvg_files that - when specified - restricts the deletion to files with a stem equal to jobname.

Motivation and Explanation: Why and how do your changes improve the library?

This change avoids removing unrelated files. In a future contribution to Manim I would like to add a class that converts TeX (containing TikZ) to SVG and also writes a file that contains specific coordinates defined in the TikZ part to a file,such that you can use these coordinates in Manim. In that case the file extension of the coordinate file is added to the whitelist, such that the TeX run does not the coordinate file. The change of this pull request avoids that the coordinate file is deleted by a subsequent unrelated TeX run.
From this perspective, this fix is a necessary fix to allow for the future extension. It does not impede the current working of Manim.

Links to added or changed documentation pages

I did not change any documentation except for the docstrings inside the code.

Further Information and Comments

None.

Reviewer Checklist

• The PR title is descriptive enough for the changelog, and the PR is labeled correctly
• If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
• If applicable: newly added functions and classes are tested

[JasonGrace2282]

I'm not sure I fully understand the motivation for this - if you want to write a file with the tikz coordinates, just give it a .coords suffix (or something similar) and then pass in .coords as a white-listed suffix.

The current approach with only deleting files with a specific stem, I believe, would also fail because-depending on the latex distribution-there may be other extra files created during the latex compilation (other than the dvi file).

[digmanwaves]

The approach you suggest would work, but is bad software engineering practice. I'll try to explain what I mean by that.
In extending manim's capabilities with new animation classes, I think it is good practice to reuse as much of the existing coding infrastructure. Therefore, I would like to reuse the existing caching system that has been setup in (a.o.) tex_file_writing.py.
The only place I can inject another file extension in the white-list is where the the delete_non_svg_files function is called. If I inject it there, then I inject it for all existing classes (that use tex_file_writing) as well. That goes against the principle of orthogonal development (i.e. avoiding cross-interactions between software components). I agree that not deleting everything but .svg and .tex files is also not-orthogonal to the current implementation, but deleting files by using a class that has not generated the files in the first place is anarchistic software development (that I can understand from Sanderson's perspective developing some personal software and "coding with minimal effort to a working target", so no blame from him, I might have done the same thing), but it has no place in software that is co-developed by multiple people.
The argument that multiple LaTeX distributions may leave different pieces of dirt laying around and that you want to clean them up, should be tackled separately and not as part of a wildcard cleanup. That said, I'm using lualatex/pdflatex/xelatex on linux and they don't leave dirt behind unless you use packages that are of no use in manim (like generating list, indices, tocs, ...).

A different, but as good approach would be to compile the tex file in a temporary directory, copying the tex and any other file extension one wants to preserve (on the whitelist) to the cache and removing the directory. That would get rid of all unnecessary files and not interfere between different classes using tex_file_writing. If you think this is a better approach, I'm willing to implement that.

[OliverStrait]

Okay so, if we are talking about software engineering, I will be blunt:

• Your changes does not increase orthogonality, quite opposite. Every func. argument is one more binding promise to outside and will make function less usefull.
  • And you just made boolean flip function, and Manim has already too much of those. These kind of functions are harder to read and reason in future (sure, in this case it is simple, but still half time useless..)
• There is hardly any reason original function to exist in first place (only one instance), it could be more usefull if it was remove_types_from_folder(file_types, ) etc. Current version is pretty much useless and mostly just separated from larger code.
• I don't know how much serious refactoring you have done in past, but detached duplicate function or even old fashion copypaste code can be better than modifying old code to flex every direction.
• This is violation of greatest rule of all, don't solve future problems. If you never come back and fill your promise, then this is only confusing orphan code for community. Why this is PR now, what value it will bring today?

These functions could have some rewriting and move those global config references etc. to inside of caller classes. Better solution is to move those deletion codes to caller classes.

[digmanwaves]

Dear Sir,
I clearly must have upset you in some way. That was not my intention. As a first-time manim contributor, I hope you appreciate that I don't refactor manim in its entirety (which would be arrogant), which is what you seem to suggest. The boolean flip function is only there not to break any current implementations. If that does not make sense to you, then apparently I entered the wrong community. Have a nice day. You can forget about the PR. I will continue on my own.
best regards,
Walter.

[behackl]

File caching as a whole is, unfortunately, just as so many other things, implemented in a rather messy and fragmented way. In an ideal world, there would be some sort of asset manager that control the life cycle of all assets generated and cached by the library, but we don't have that at the moment, nor is it worth to delay future developments for it.

The feature you describe does sound interesting; better interaction for TikZ drawings, in particular the ability to have better coordinate-wise control over them, does sound interesting, and I encourage you to open an issue with more details on the interface you imagine to get feedback from the community. This would also strengthen the case for merging this change here; seen in isolation it is hard to see the benefit of the somewhat semantic nature of the improvement of the cleanup behavior vs. the drawback of the improved complexity in the implementation.

Alternatively, it might make sense to spend the additional epsilon of effort and think about replacing the current delete_nonsvg_files function (i.e., with your change the calls without a specified jobname argument) with a global cleanup function, and use delete_nonsvg_files exclusively in the context of cleaning up auxiliary files of a single given TeX compilation run. (I did not check whether any further calls to delete_nonsvg_files are currently made, so this last paragraph might also just not apply; in this case the jobname argument should be mandatory).

[behackl]

I'd still be interested in hearing more about your plans @digmanwaves, but it appears this PR is stale and I'll close it for now.