aiidateam / aiida-core

The official repository for the AiiDA code
https://aiida-core.readthedocs.io
Other
431 stars 187 forks source link

📝 Integrate updated cheat sheet into main docs #6332

Closed GeigerJ2 closed 5 months ago

GeigerJ2 commented 6 months ago

This PR integrates an updated version of the cheat sheet into the intro section of the main docs.

The main changes to the SVG file are:

In the rst file, a link to the SVG is included, so that the figure is displayed directly on the page (I didn't manage to integrate the SVG source code directly into the rst, but I also don't think it's necessary). Clicking on the image opens it directly in the browser. A download link to the PDF in the typical 2-page layout for printing is also included.

One could add more hyperlinks, but I think for now it's fine. And before I continue to frantically align SVG elements, better to get it out there. Lastly, please rebase before merge :D

Pinging @khsrali, @sphuber, @mbercx and @giovannipizzi for feedback if anybody has the time. Thanks!

GeigerJ2 commented 6 months ago

Thanks for the feedback, @sphuber! Regarding your points:

What are the other PDF versions for (cheatsheet1.pdf and cheatsheet2.pdf)?

These were just intermediate files and can be removed. As the layout in the SVG is horizontal, I exported each page separately and merged them with pdftk.

Why cheatsheet_h.svg and cheatsheet_v.pdf?

Same reason here. I just had the labels as I experimented with multiple files. I'll remove them, so it'll just be cheatsheet.pdf and cheatsheet.svg.

The hitboxes of the links on the second line in the "additional web resources" are very small. Most part of the text is not clickable. Can this be fixed?

Hm, that's weird. The whole text elements should be clickable. In the PDF, for me, it's fine (aka it works on my machine). I'll make the text and boxes into groups and set the anchor there, that should fix it and correspond better with the elements being clickable buttons. Did you try it with the PDF locally or with the SVG in the browser?

It would be good if code can be put in a monospaced font

It should already be? At least in the text boxes, code is monospaced (Ubuntu Mono font). Do you mean in the graphs?

Fix what went astray: should really be verdi daemon stop\nverdi process repair\nverdi daemon start.

Alright, updated!

"ProcessNode attributes": there is still ".etc?"

Ah, yes, I see how that might be confusing. The reason for "etc." was that these are not all the properties, and I added the question mark as the code is phrased like "is finished?", "is stored?" :D I'll replace it with a "..."

verdi storage list is not an actual command

Must've been a typo.

verdi calcjob/process dump don't exist, yet...

They should be there in a few days!

I noticed that you included the version number in the cheatsheet. Although that might be useful to have, it is for sure that we are going to forget to update it. Is there a way we can automatically update it? Otherwise maybe it is best to remove it?

A bit pessimistic, but probably an accurate assessment :D That was just added as it was included in the previous version of the cheat sheet. Though, as part of the aiida-tutorials that was also more static. One could probably update it automatically, but in the case that it becomes actually relevant, i.e. if a newer AiiDA version changes the API, then we'd also have to update that in the cheat sheet, not just the version number. So, for me, it's also fine to remove it.

sphuber commented 6 months ago

It should already be? At least in the text boxes, code is monospaced (Ubuntu Mono font). Do you mean in the graphs?

I just checked and the PDF does render correctly, but the svg in the browser doesn't show the monospaced fonts:

image

Not sure if that is a problem of the SVG or the OS/browser. But I see the same behavior on both Firefox and Edge on Windows 11.

GeigerJ2 commented 6 months ago

Not sure if that is a problem of the SVG or the OS/browser. But I see the same behavior on both Firefox and Edge on Windows 11.

Then it might be that the font (Ubuntu Mono) is not available on Windows. I'll look for a monospaced font that is available on both systems. Alternatively, one could also link the exported PDF instead of the SVG.

sphuber commented 5 months ago

Do you want to wrap this one up @GeigerJ2 ? I think for me the open action points are:

Then this can be merged from my side. Thanks

GeigerJ2 commented 5 months ago

Yeah, in principle this can be wrapped up. I just need to make sure the chosen monospaced font works on all OSs, or (and this might be my preferred solution) I just link the rendered PDF rather than the SVG. I think linking the SVG in the rst file was just done as the original idea was to directly implement the vector graphic in the rendered page, but if it works by just linking the compiled PDF, no need for extra headache. I have all the other changes applied locally, just holding off as verdi process/calcjob dump isn't merged yet, and it's contained in the cheat sheet.

GeigerJ2 commented 5 months ago

Just to add here the reason for having the cheat sheet in both horizontal and vertical layout: For displaying on screen, I think horizontal layout is better, as one can see both pages directly at a glance (integrated into the rst), while for printing a vertical two-page layout might be preferred, as it allows for duplex printing (when following the download link).

sphuber commented 5 months ago

@GeigerJ2 having trouble with git rebase? 😅

GeigerJ2 commented 5 months ago

@GeigerJ2 having trouble with git rebase? 😅

Yeah, I think it's because I'm using git worktrees, and rebasing the branch from the respective git worktree folder causes this mess... Almost convinced to ditch the approach, but I don't want to give @mbercx this victory yet :D

GeigerJ2 commented 5 months ago

Never happened, nothing to see here, just added one commit 👀 (Well, now the other commits are missing. I'll have another look tomorrow.)

GeigerJ2 commented 5 months ago

Alright, this should be ready for merge, and I think it would be nice to include in the v2.6 release. I addressed your comments, @sphuber, could you please take it for another spin?

On the page, it's now showing the image as PNG (embedding the PDF didn't work easily without using raw HTML; and neither did this work as I intended). The image target points to the PDF in horizontal layout for nice display on the screen, meaning that fonts should show correctly irrespective of OS and browser, and the hyperlinks should work. I had to add this PDF to _static, as it's not directly referenced via image, so it's not being copied over to build/html/_images otherwise. The download link points to the PDF in vertical, two-page layout to allow for two-sided printing. I left the labels _h and _v for the two PDFs to avoid confusion. While this requires adding two PDF files, I think the horizontal on-screen display and the two-page download would be nice to have. I removed verdi archive dump, as that's a feature that we anticipate and will start working on soon, but it won't make it into this release.

Lastly, can we clean up/squash the commits (i.e. to remove the merge-commit, as to not infuriate @mbercx) when doing the merge, or does that require an interactive rebase locally (still having PTSD from my former two fuck-ups :D). Thanks!