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

CHORE: Use f-string instead of format #5272

Merged
merged 33 commits into from
Oct 22, 2024
Merged

Conversation

gmalinve
Copy link
Contributor

No description provided.

@gmalinve gmalinve self-assigned this Oct 10, 2024
@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@gmalinve gmalinve linked an issue Oct 10, 2024 that may be closed by this pull request
@github-actions github-actions bot added the testing Anything related to testing label Oct 10, 2024
Copy link
Collaborator

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

Is there a need to leverage the following snippet "'\" some string \"'" ? I think it would be easier to read with '" some string "'. I'm asking because it is appearing in multiple places and it is hard to read in some cases

_unittest/conftest.py Outdated Show resolved Hide resolved
doc/source/Resources/pyaedt_installer_from_aedt.py Outdated Show resolved Hide resolved
doc/source/Resources/pyaedt_installer_from_aedt.py Outdated Show resolved Hide resolved
@gmalinve gmalinve marked this pull request as ready for review October 14, 2024 16:21
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 69.26995% with 181 lines in your changes missing coverage. Please review.

Project coverage is 83.34%. Comparing base (3ca765f) to head (37f344d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5272      +/-   ##
==========================================
+ Coverage   83.28%   83.34%   +0.06%     
==========================================
  Files         143      143              
  Lines       59151    59151              
==========================================
+ Hits        49261    49299      +38     
+ Misses       9890     9852      -38     

Copy link
Collaborator

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

Thanks for this HUGEEEEEE refactoring, it will help a lot with future maintenance !

I found some typos that need to be adressed (one seem to be already here before your chagnes but I might be wrong).
Also, why did you perform extra changes in the tests (not only related to f-string formating) ?

_unittest/test_02_2D_modeler.py Outdated Show resolved Hide resolved
_unittest/test_11_Setup.py Show resolved Hide resolved
_unittest/test_12_PostProcessing.py Show resolved Hide resolved
src/ansys/aedt/core/application/analysis.py Show resolved Hide resolved
src/ansys/aedt/core/application/analysis.py Outdated Show resolved Hide resolved
src/ansys/aedt/core/application/variables.py Show resolved Hide resolved
src/ansys/aedt/core/circuit.py Outdated Show resolved Hide resolved
src/ansys/aedt/core/desktop.py Outdated Show resolved Hide resolved
src/ansys/aedt/core/generic/general_methods.py Outdated Show resolved Hide resolved
gmalinve and others added 3 commits October 18, 2024 11:28
Co-authored-by: Sébastien Morais <146729917+SMoraisAnsys@users.noreply.github.com>
Co-authored-by: Sébastien Morais <146729917+SMoraisAnsys@users.noreply.github.com>
Copy link
Contributor

@anur7 anur7 left a comment

Choose a reason for hiding this comment

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

@gmalinve I reviewed only first 50 files! By the way, very impressive you went through all of these files! If I have time, I'll look at the rest.

_unittest/test_01_configuration_files.py Outdated Show resolved Hide resolved
_unittest/test_22_Circuit_DynamicLink.py Show resolved Hide resolved
src/ansys/aedt/core/filtersolutions_core/__init__.py Outdated Show resolved Hide resolved
src/ansys/aedt/core/generic/general_methods.py Outdated Show resolved Hide resolved
gmalinve and others added 4 commits October 21, 2024 07:39
# Conflicts:
#	src/ansys/aedt/core/visualization/plot/matplotlib.py
#	src/ansys/aedt/core/visualization/post/solution_data.py
Co-authored-by: Abdun Nur <146203416+anur7@users.noreply.github.com>
The expected behavior of this method is not
clean enough and the changes cause an issue with flake8.
Let us leave this as it is.
Copy link
Contributor

@anur7 anur7 left a comment

Choose a reason for hiding this comment

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

@gmalinve finished to review! Good job :)

@gmalinve gmalinve merged commit c4e8562 into main Oct 22, 2024
40 of 41 checks passed
@gmalinve gmalinve deleted the chore/remove_format_string branch October 22, 2024 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Anything related to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

REFACTOR: Leverage f-string instead of format
4 participants