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

Enable LUT selection for emodulus computation in analysis view #166

Merged

Conversation

RaghavaAlajangi
Copy link
Contributor

This PR aims to implement the selection of LUT in the analysis view. This is mentioned as a second point in the issue #71.

self.comboBox_visc_model.setVisible(medium_key.count("MC-PBS"))
self.comboBox_lut.setVisible(medium_key.count("MC-PBS"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know whether we need this. I just followed the comboBox_visc_model implementation.

Copy link
Member

Choose a reason for hiding this comment

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

The viscosity model is only applicable for MC-PBS, whereas the LUT selection is universal. So no, we must not hide the LUT selection if the buffer is set to MC-PBS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for letting me know.

@RaghavaAlajangi
Copy link
Contributor Author

Hi @paulmueller
please take a look at these changes when you have time.

Copy link

codecov bot commented Sep 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.26%. Comparing base (981290c) to head (b941a79).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
+ Coverage   79.22%   79.26%   +0.04%     
==========================================
  Files          64       64              
  Lines        6974     6988      +14     
==========================================
+ Hits         5525     5539      +14     
  Misses       1449     1449              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -143,6 +145,9 @@ def __setstate__(self, state):
# use defaults from previous session (Herold-2107)
idx_vm = 1
self.comboBox_visc_model.setCurrentIndex(idx_vm)
# Set current state of the emodulus lut
idx_lut = self.comboBox_lut.findText(emodulus.get("emodulus lut", ""))
Copy link
Member

Choose a reason for hiding this comment

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

Just a comment: For now, this is fine, because it follows the current code. But in general, using Data instead of Text is cleaner, because text might be subject to internationalization: #167

self.comboBox_visc_model.setVisible(medium_key.count("MC-PBS"))
self.comboBox_lut.setVisible(medium_key.count("MC-PBS"))
Copy link
Member

Choose a reason for hiding this comment

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

The viscosity model is only applicable for MC-PBS, whereas the LUT selection is universal. So no, we must not hide the LUT selection if the buffer is set to MC-PBS.

@RaghavaAlajangi
Copy link
Contributor Author

Hi @paulmueller ,
I have made the changes that are asked for. Please have a look.

@paulmueller paulmueller merged commit 478d10e into ZELLMECHANIK-DRESDEN:master Sep 24, 2024
5 checks passed
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