Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[SCFD-4128] [SCFD-4126] feat(): Removed validation process for unit system conversion #666

Merged
merged 8 commits into from
Jan 23, 2025

Conversation

benflexcompute
Copy link
Collaborator

@benflexcompute benflexcompute commented Jan 17, 2025

  • feat(): Removed validation process for unit system conversion
  • Change temperature unit for imperial to be Fahrenheit

@benflexcompute benflexcompute marked this pull request as draft January 17, 2025 20:30
@benflexcompute benflexcompute changed the title feat(): Removed validation process for unit system conversion [SCFD-4128] [SCFD-4126] feat(): Removed validation process for unit system conversion Jan 17, 2025
@@ -58,6 +60,7 @@
)
from flow360.component.utils import remove_properties_by_name
from flow360.exceptions import Flow360TranslationError
from flow360.log import log
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove this import since it is not used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, curse the pytlint disable. We should limit its scope from now on.

def test_unit_system_conversion():
with open("data/simulation_param.json", "r") as fp:
dict_to_convert = json.load(fp)
services.change_unit_system(data=dict_to_convert, new_unit_system="Imperial")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we also need to test the conversion to "CGS" and "SI"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added to CGS conversion and SI conversion.

@benflexcompute benflexcompute force-pushed the BenY/NoValidationUnitConversion branch from 08d1a5b to b754b83 Compare January 22, 2025 15:25
Change temperature unit for imperial to be Fahrenheit

Fixed bugs

More tests
@benflexcompute benflexcompute force-pushed the BenY/NoValidationUnitConversion branch from b754b83 to f3d72c2 Compare January 22, 2025 21:11
@benflexcompute benflexcompute self-assigned this Jan 22, 2025
@benflexcompute benflexcompute marked this pull request as ready for review January 22, 2025 21:18
@benflexcompute benflexcompute merged commit 05b38de into develop Jan 23, 2025
15 checks passed
@benflexcompute benflexcompute deleted the BenY/NoValidationUnitConversion branch January 23, 2025 03:26
benflexcompute added a commit that referenced this pull request Jan 23, 2025
…ystem conversion (#666)

* feat(): Removed validation process for unit system conversion

Change temperature unit for imperial to be Fahrenheit

Fixed bugs

More tests

* Ready to be reviewed

* Fix lint

* Fix lint

* Corrected the SI unit test

* Fix fragial windows float computation
benflexcompute added a commit that referenced this pull request Jan 23, 2025
…ystem conversion (#666) (#679)

* feat(): Removed validation process for unit system conversion

Change temperature unit for imperial to be Fahrenheit

Fixed bugs

More tests

* Ready to be reviewed

* Fix lint

* Fix lint

* Corrected the SI unit test

* Fix fragial windows float computation
benflexcompute added a commit that referenced this pull request Jan 23, 2025
benflexcompute added a commit that referenced this pull request Jan 23, 2025
…use they are not tested yet and I need to deploy a new version. (#684)

* Revert "fix(): Ammend bugs in PR#668, also changed Imperial unit system to use Fahrenheit as default temperature unit (#678) (#680)"

This reverts commit 160062e.

* Revert "[SCFD-4128] [SCFD-4126] feat(): Removed validation process for unit system conversion (#666) (#679)"

This reverts commit 2568a16.

* Revert "fix(): temperature_offset has same unit conversion as temperature (#668) (#677)"

This reverts commit 07b91cc.

* Hacy fix to diable the positive temperature check.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants