KonstantinosRekoumis / CSR

A primitive calculator that calculates a ship midship plates and PSMs based on IACS CSR
2 stars 2 forks source link

Decouple LaTeX code from Python #4

Open gerelef opened 9 months ago

gerelef commented 9 months ago

Issue

Currently, tens of LaTeX loc are embedded in python; they are not only hard to edit, but they obsfuscate the code, detracting from focusing on what's important while fixing bugs and/or refactoring. Moreover, the current approach is less extendable, and more prone to bugs due to being hard to statically analyse with external tools.

Proposal

Move appropriate latex code segments from being embedded in python code, to a dedicated resource directory for application files.

gerelef commented 9 months ago

I was looking at latex.py which genuinely scares me, because I can't fix it in any hacky way I try. However, I stumbled upon this answer on stackoverflow which looks pretty legit; the only real usecase I see in this project is simply replacing specific values in a preset LaTeX template. Here's an excerpt of an entire section of latex.py

'$L_{BP}$ '+f'&{ship.LBP}&'+' [m]\\tabularnewline \\hline\n'
'$L_{sc} = L$ '+f'&{ship.Lsc}&'+' [m]\\tabularnewline \\hline\n'
'$B$ '+f'&{ship.B}&'+' [m]\\tabularnewline \\hline\n'
'$T$ '+f'&{ship.T}&'+' [m]\\tabularnewline \\hline\n'
'$T_{min}$ '+f'&{ship.Tmin}&'+' [m]\\tabularnewline \\hline\n'
'$T_{sc}$ '+f'&{ship.Tsc}&'+' [m]\\tabularnewline \\hline\n'
'$D$ '+f'&{ship.D}&'+' [m]\\tabularnewline \\hline\n'
'$C_b$ '+f'&{ship.Cb}&'+' \\tabularnewline \\hline\n'
'$C_p$ '+f'&{ship.Cp}&'+' \\tabularnewline \\hline\n'
'$C_m$ '+f'&{ship.Cm}&'+' \\tabularnewline \\hline\n'
'$DWT$ '+f'&{ship.DWT}&'+' \\tabularnewline \\hline\n'

To me, this looks like a textbook case for builtin Template, and with this we could move everything LaTeX into a few files, read them in their entirety and subsistute the appropriate values. This wouldn't be the most maintainable thing in the world as a naive implementation, but it would be miles beyond what we have right now. But perhaps this is not enough, I'm looking for feedback.

KonstantinosRekoumis commented 9 months ago

Yeap, seems about right. The line can be templated as follows:

line = lambda name, val, mu: f"${name}$&{val}&{mu}\\tabularnewline \\hline\n"

And iterate over the required elements Have in mind that the Template solution may break because of the $ required by the LaTeX code.

gerelef commented 9 months ago

I was thinking of extracting the entire text to a text file, and using that as a template instead of fstrings? in your example you're using fstrings, not the templating "engine."

gerelef commented 9 months ago

According to this stackexchange answer instead of $ for inline math delimiters, we can use \(...\)

edit: apparently this is somewhat controversial, but the discussion regarding which is best is leaning towards the latter

gerelef commented 9 months ago

Apparently, we can also change the delimiter this way

>>> class CustomTemplate(Template):
...     delimiter = '^^^'
... 
>>> s = CustomTemplate(r"^^^who likes ^^^what")
>>> print(s.substitute(who="tim", what="pasta"))
tim likes pasta

What's your proposal to use as a delimiter? We need something that'll definitely never appear in our template.

gerelef commented 9 months ago

I've done some preliminary work, however standalone flag behaviour is not implemented. Is standalone required or should we remove it? From what I gather, the preamble is only used when producing this document, so it probably shouldn't be in it's own file. The mid section of the document is not used anywhere else from what I gather either.

\documentclass[12pt,a4paper]{report}

\usepackage[a4paper,headheight =15pt]{geometry}
\usepackage{array}
\usepackage{multirow}
\usepackage{longtable}
\usepackage{pdflscape}
\usepackage{amsmath}
\usepackage{comment}
\usepackage{caption}
\usepackage{graphicx}
\usepackage{fancyhdr}
\usepackage{typearea}
\usepackage[absolute]{textpos}

\fancypagestyle{normal}{\fancyhf{}\rhead{\thepage}\lhead{\leftmark}\setlength{\headheight}{15pt}
\renewcommand{\headrulewidth}{1pt}
\renewcommand{\footrulewidth}{0pt}}
\fancypagestyle{lscape}{%
\fancyhf{} % clear all header and footer fields
\fancyhead[L]{%
\begin{textblock}{0}(1,12){\rotatebox{90}{\underline{\leftmark}}}\end{textblock}
\begin{textblock}{2}(1,1){\rotatebox{90}{\underline{\thepage}}}\end{textblock}}
\setlength{\headheight}{15pt}
\setlength{\footheight}{0pt}
\renewcommand{\headrulewidth}{0pt}
\renewcommand{\footrulewidth}{0pt}}
\graphicspath{{./}}
% preamble end

\chapter{General Input Data Particulars}
\label{ch:general-particulars}
\begin{table}[h]
\caption{Ship\'s General Input Data Particulars}
\label{tab:Gen_Part}
\begin{tabular}{{>{\centering}m{6cm}}*{2}{>{\centering}m{4cm}}}
\hline
\(L_{BP}\) &^^^LBP& [m]\tabularnewline \hline
\(L_{sc} = L\) &^^^Lsc& [m]\tabularnewline \hline
\(B\) &^^^B& [m]\tabularnewline \hline
\(T\) &^^^T& [m]\tabularnewline \hline
\(T_{min}\) &^^^Tmin& [m]\tabularnewline \hline
\(T_{sc}\) &^^^Tsc& [m]\tabularnewline \hline
\(D\) &^^^D& [m]\tabularnewline \hline
\(C_b\) &^^^Cb& \tabularnewline \hline
\(C_p\) &^^^Cp& \tabularnewline \hline
\(C_m\) &^^^Cm& \tabularnewline \hline
\(DWT\) &^^^DWT& \tabularnewline \hline
k (material factor) &^^^kappa& [m]\tabularnewline \hline % FIXME ship.kappa : 0.3g
\(M_{wh}\) &^^^Mwh& [kNm]\tabularnewline \hline
\(M_{ws}\) &^^^Mws& [kNm]\tabularnewline \hline
\(M_{sw,h-mid}\) &^^^Msw_h_mid& [kNm]\tabularnewline \hline
\(M_{sw,s-mid}\) &^^^Msw_s_mid& [kNm]\tabularnewline \hline
\(C_w\) &^^^Cw& \tabularnewline \hline
\(y_{neutral}\) &^^^yo& [m]\tabularnewline \hline
\(I_{net,\, v}\) &^^^Ixx& [\(m^^^4\)]\tabularnewline \hline
\(I_{n-50,\, v}\) &^^^n50_Ixx& [\(m^^^4\)]\tabularnewline \hline
\(a_0\)   &^^^a0& \tabularnewline \hline
\end{tabular}
\end{table}
% general part end

% FIXME for fig in FIGS
%  perhaps move this into it's own template, copy multiple times & replace in THIS document at THIS position?
%\begin{figure}[h]
%\centering
%\includegraphics[width=\linewidth]{'
%f'{i}'
%}\end{figure}
% figures end

\newgeometry{margin=1.5cm}
\chapter{Pressure Data}
\label{ch:pressure_data}+text[0]
% pressure end

\chapter{Plating Data}
\label{ch:plating_data}
\newpage
\thispagestyle{lscape}
\pagestyle{lscape}
\begin{landscape}
^^^plating_data
\end{landscape}
\thispagestyle{normal}
\pagestyle{normal}
% plates end

\chapter{Stiffeners Data}
\label{ch:stiffeners_data}
\newpage
\thispagestyle{lscape}
\pagestyle{lscape}
\begin{landscape}
^^^stiffeners_data
\end{landscape}
\thispagestyle{normal}
\pagestyle{normal}
% stiffeners end

\clearpage
\chapter{Stiffened Plates Data}
\label{ch:stiffened_plates_data}
^^^stiffened_plates_data
% stiff plates end

\chapter{Ordinary Section\'s Stiffened Plates Data}
\label{ch:stiffeners-data}
^^^stiffeners_symmetrical_disclaimer
^^^stiffeners_data
% ordinary section end

\clearpage
\restoregeometry
% postfix
gerelef commented 9 months ago

In f(a) of DataLogger, there's a Logger.warning at the end of the if elif else. Isn't that hiding a possible bug, and as such shouldn't that be a Logger.error?

KonstantinosRekoumis commented 9 months ago

Apparently, we can also change the delimiter this way

>>> class CustomTemplate(Template):
...     delimiter = '^^^'
... 
>>> s = CustomTemplate(r"^^^who likes ^^^what")
>>> print(s.substitute(who="tim", what="pasta"))
tim likes pasta

What's your proposal to use as a delimiter? We need something that'll definitely never appear in our template.

yeap I don't think that the ^^^ sequence is encountered in LateX code, so it is a good delimiter to use. % is used for comments and maybe a combination of ^%^ is better because it is for sure a sequence that will not be encountered and if encountered will not make any sense in equational context.

I've done some preliminary work, however standalone flag behaviour is not implemented. Is standalone required or should we remove it? From what I gather, the preamble is only used when producing this document, so it probably shouldn't be in it's own file. The mid section of the document is not used anywhere else from what I gather either.

Standalone flag is mandatory because the output-report code should be able to be implemented into an already existing LaTeX report without requiring the user to manually delete the preamble and other standalone report features that would be implemented (ie. List of Figures, Table of Contents, etc). Standalone behavior was broken during the latest refactoring I did, and should be amended.

gerelef commented 9 months ago

Standalone behavior was broken during the latest refactoring I did, and should be amended.

Please open an issue so we don't forget about it.

gerelef commented 9 months ago

With #31 merged, this issue is partially done. As mentioned, only the refactor of datalogger.py is remaining.

gerelef commented 9 months ago

@KonstantinosRekoumis The only point for latex.py output_latex to have a standalone (now called embeddable) mode, is to be modular so that it can be integrated within another document. If someone attempts to do so currently, the output will be erroneous unless they have the same imports & macros we do beforehand. This means this is non-portable currently, even if we do output an "embeddable" version. Shouldn't dependencies and macros be defined on the top of our embeddable segment, instead of the preamble, in case it's standalone?

KonstantinosRekoumis commented 9 months ago

Maybe raise a Warning to the user and add the dependencies to the README.md. The point is that comments in LaTeX are part of a package, so inputting the dependencies as comments in the embeddable section may lead to erroneous behavior as well. LaTeX moment...

gerelef commented 9 months ago

As far as I know, packages are embeddable in the document section as well, not only in the preamble. We should consider as if to modify the preamble.tex and content.tex templates appropriately to include the dependencies as needed. As far as I could find, Latex deals with duplicate packages pretty well, so there's no custom handling we'd need even if we were to import the packages twice.