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

Refactor of template files #55

Merged
merged 97 commits into from
Apr 18, 2024
Merged

Refactor of template files #55

merged 97 commits into from
Apr 18, 2024

Conversation

jteijema
Copy link
Member

@jteijema jteijema commented Mar 29, 2024

This PR:

  • Adds a base class for templates and moves duplicate code to the base class
  • passes wordcloud flag to docs renderer
  • adds allow overwrite flag
  • adds a catch for using the wrong template with the wrong template class
  • cleans up name passing (name is now only in the template class, or the actual template, no more random strings)
  • cleans up valid template checker
  • cleans up the console output
  • format the documents using ruff
  • add an extra line to the filehandler writer, that way we can remove all the double empty lines in all templates
  • update the workflow
  • add a config DEFAULT object
  • add prohibited arguments to templates
  • refactor platform detection code
  • refactor fp_template code
  • n_runs only adds a _{{ run }} to the filename if n_runs is more than 1

I also structured a response to missing parameters. Since we assume our templates are correct, only custom templates can have missing parameters (parameters used in the template but not provided by the python class). Therefore, we can throw an error and assume the user is using the wrong base template with their custom template.

I've added a catch for if a user selects a base template that's not part of the 3 predefined base templates. Not possible under normal use, but if an extension adds a template to the base templates, this could happen.

Now the pipeline is:

  1. cast base template to lower case
  2. check if base template exists (if not, throw an error)
  3. check if a custom template path is given
  4. check if custom template path is valid
  5. load the datasets
  6. start the rendering

@jteijema jteijema marked this pull request as draft March 29, 2024 14:51
@jteijema jteijema requested a review from J535D165 March 29, 2024 14:51
@jteijema jteijema marked this pull request as ready for review March 30, 2024 00:40
@PeterLombaers
Copy link
Member

The README could probably use an update as well, in particular the section on making a custom template. You could now also do that via an extension, and using the TemplateBase class.

@PeterLombaers
Copy link
Member

PeterLombaers commented Apr 18, 2024

Everything seems to work well! The following comments I made could also be handled in separate pull requests.

asreviewcontrib/makita/entrypoint.py Outdated Show resolved Hide resolved
asreviewcontrib/makita/entrypoint.py Outdated Show resolved Hide resolved
asreviewcontrib/makita/template_arfi.py Outdated Show resolved Hide resolved
asreviewcontrib/makita/template_arfi.py Outdated Show resolved Hide resolved
@jteijema jteijema merged commit 1fcda57 into asreview:main Apr 18, 2024
4 checks passed
@jteijema jteijema deleted the base-template-class branch April 18, 2024 13:10
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.

2 participants