Conversation
| if value is None: | ||
| return '' | ||
| if isinstance(value, float) and math.isnan(value): | ||
| return '' |
There was a problem hiding this comment.
I think instead of writing this logic by hand, it would be better to use tabulate with dataframes (both dependencies already in the project). You can then do df.to_markdown
| </span> | ||
| </label> | ||
|
|
||
| <CoverPagePreview |
There was a problem hiding this comment.
this is more just an idea - you know the usecase better than me, but couldn't we merge this into the template - making each template either have it or not instead of specifying every time you run the dmp generation?
There was a problem hiding this comment.
Based on our meeting, we agreed to move the cover page checkbox to the template form.
| value_key: description | ||
| preview: Named project versions and their descriptions, newest first. Empty if no versions are available. | ||
|
|
||
| sections: |
There was a problem hiding this comment.
I think maybe we should add docs. I know there are parts of the project I worked on which should be documented and are not (such as the queueing mechanism). And i think this could also use documentation, because I have no idea what I am supposed to set here if I was someone trying to deploy this
| sections: | ||
| - id: projects | ||
| title: Projects | ||
| content: >- |
There was a problem hiding this comment.
I would prefer to move this part into the user created template.
When creating a new template, users would be able to "add proects page"", it would then pre-fill the title and description for them, they would be able to edit it.
Upsides is that you would simplify the backend part since this change only touches frontend (don't know how we would handle multilingual support, maybe we would always use english for this part since the language is selected later anyway.)
Downside is that it would be harder to insert the page break. We would need to have an option to add page breaks into the template.
We can have a call about this and discuss
There was a problem hiding this comment.
Based on our meeting, we agreed to move the cover page checkbox to the template form.
|
|
||
| async def translate( | ||
| self, | ||
| assignments: list[SerializedSectionAssignment], |
There was a problem hiding this comment.
is the translation necessary? Will user ever see the template text? If not, I would skip the translation, LLM should not care about the language of the template, it will simply produce the language specified by the user when generating based on a template. It does not matter what language the template is written in.
There was a problem hiding this comment.
or, if we keep it, it would be nice to run it in parallel with polishing to save time
There was a problem hiding this comment.
Based on our meeting, we agreed to keep the cover page in English and address the language setting more generally in a future issue.
| ) | ||
| serializable = [assignment.to_dict() for assignment in assignments] | ||
| serializable = ( | ||
| existing_assignments |
There was a problem hiding this comment.
I would rather do explicit if existing_assignments is not None else ...
This is hard to read
| 'created_at': statement.excluded.created_at, | ||
| 'assignments': statement.excluded.assignments, | ||
| **( | ||
| {'content_assignments': statement.excluded.content_assignments} |
There was a problem hiding this comment.
I dont like the way this looks, but maybe writing it in another more simple if/else way would take up too many lines :D
2 participants
cover-pagedirectory.EDITpermissions, as agreed with Marek.Closes #119
Cover page translations moved to branch https://github.com/ds-wizard/ai-document-plugin/tree/feature/119-backup-cover-page-translation