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

Text review and update #281

Merged
merged 10 commits into from
Sep 20, 2022
Merged

Text review and update #281

merged 10 commits into from
Sep 20, 2022

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Sep 19, 2022

fixes #277

Update text on feedback from Nicola.

@unkcpz unkcpz added this to the 2nd Live testing with Nicola milestone Sep 20, 2022
Copy link
Member

@mbercx mbercx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @unkcpz! I've left a few comments.

@@ -28,7 +28,9 @@ class WorkChainSelector(ipw.HBox):
FMT_WORKCHAIN = "{wc.pk:6}{wc.ctime:>10}\t{wc.state:<16}\t{wc.formula} \t {wc.relax_info} \t {wc.properties_info}"

def __init__(self, **kwargs):
self.work_chains_prompt = ipw.HTML("<b>Select workflow or start new:</b>&nbsp;")
self.work_chains_prompt = ipw.HTML(
"<b>Select computed workflow or start a new:</b>&nbsp;"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd still add "one" here at the end ("start a new one"). But the sentence is already taking up quite a bit of space as it is 😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot of space for this line, but I still prefer without 'one' which is more straightforward.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this is grammatically incorrect?

The <a href="https://www.quantum-espresso.org/" target="_blank">Quantum ESPRESSO</a> app (or QE app for short) is a graphical interface for calculating material properties based on density-functional theory (DFT).
Each of the properties is calculated by workflows powered by the <a href="https://www.aiida.net/" target="_bland"> AiiDA engine</a>, and maintained in the <a href="https://aiida-quantumespresso.readthedocs.io/en/latest/" target="_blank"> Quantum ESPRESSO plugin</a> for AiiDA.
The <a href="https://www.quantum-espresso.org/" target="_blank">Quantum ESPRESSO</a> app (or QE app for short) is a graphical front end for calculating materials properties using Quantum ESPRESSO (QE).
Each of the property is calculated by workflows powered by the <a href="https://www.aiida.net/" target="_bland"> AiiDA engine</a>, and maintained in the <a href="https://aiida-quantumespresso.readthedocs.io/en/latest/" target="_blank"> Quantum ESPRESSO plugin</a> for AiiDA.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems incorrect? Maybe "Each property"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed

aiidalab_qe/steps.py Outdated Show resolved Hide resolved
aiidalab_qe/steps.py Outdated Show resolved Hide resolved
Copy link
Member Author

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbercx thanks! I am for sure not able to judge which phrase is better because of my poor ENG writing 😆 But I think we can loose a bit, I accept some requests and it is good to go I guess.

@@ -28,7 +28,9 @@ class WorkChainSelector(ipw.HBox):
FMT_WORKCHAIN = "{wc.pk:6}{wc.ctime:>10}\t{wc.state:<16}\t{wc.formula} \t {wc.relax_info} \t {wc.properties_info}"

def __init__(self, **kwargs):
self.work_chains_prompt = ipw.HTML("<b>Select workflow or start new:</b>&nbsp;")
self.work_chains_prompt = ipw.HTML(
"<b>Select computed workflow or start a new:</b>&nbsp;"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot of space for this line, but I still prefer without 'one' which is more straightforward.

aiidalab_qe/steps.py Outdated Show resolved Hide resolved
Co-authored-by: Marnik Bercx <mbercx@gmail.com>
@unkcpz unkcpz requested a review from mbercx September 20, 2022 10:10
Co-authored-by: Marnik Bercx <mbercx@gmail.com>
Copy link
Member

@mbercx mbercx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good! 🚀

@unkcpz unkcpz merged commit 9d927a7 into master Sep 20, 2022
@unkcpz unkcpz deleted the fix/277/nicola-text-feedback branch September 20, 2022 10:28
Copy link
Member

@csadorf csadorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've found a few issues, maybe we can fix them in a separate PR.

@@ -28,7 +28,9 @@ class WorkChainSelector(ipw.HBox):
FMT_WORKCHAIN = "{wc.pk:6}{wc.ctime:>10}\t{wc.state:<16}\t{wc.formula} \t {wc.relax_info} \t {wc.properties_info}"

def __init__(self, **kwargs):
self.work_chains_prompt = ipw.HTML("<b>Select workflow or start new:</b>&nbsp;")
self.work_chains_prompt = ipw.HTML(
"<b>Select computed workflow or start a new:</b>&nbsp;"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this is grammatically incorrect?

@@ -43,8 +43,7 @@ class PseudoFamilySelector(ipw.VBox):
)
dft_functional_help = ipw.HTML(
"""<div style="line-height: 140%; padding-top: 10px; padding-bottom: 10px; opacity:0.5;">
The exchange-correlation energy is calculated based on the charge
density using this functional. We currently provide support for two
The exchange-correlation energy is calculated functional. We currently provide support for two
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exchange-correlation energy is calculated functional.

That sounds weird to me. Should it say "is calculated via a functional" ?

accuracy and speed. Choose the "fast" protocol for a faster calculation
with less precision and the "precise" protocol that provides more
accuracy but will take longer.</div>"""
with less precision and the "precise" protocol to aim at best accuracy (at the prece of longer/costlier calculations).</div>"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: "prece"

@@ -165,7 +168,8 @@ class SmearingSettings(ipw.VBox):
smearing_description = ipw.HTML(
"""<p>
The smearing type and width is set by the chosen <b>protocol</b>.
Tick the box to override the default.
Tick the box to override the default, not advised unless you've mastered <b>smearing effects</b> (click <a href="http://theossrv1.epfl.ch/Main/ElectronicTemperature"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is "HERE" block-capitalized?

@unkcpz
Copy link
Member Author

unkcpz commented Sep 20, 2022

Thanks @csadorf, they are addressed by #296

unkcpz added a commit that referenced this pull request Sep 20, 2022
Supplementary and fixes for #281
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.

QeApp GUI text improvement feedback from Nicola
3 participants