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

rename i and j in power transformer transform & inverse transform #444

Merged

Conversation

veni-vidi-vici-dormivi
Copy link
Collaborator

Renamed i and j indexes to gridcell and year for better readability in PowerTransformerVariableLambda.transform() and inverse_transform.

@veni-vidi-vici-dormivi
Copy link
Collaborator Author

veni-vidi-vici-dormivi commented May 13, 2024

Sorry about yet another PR but I had to do this before I forget about it.
Should be easy to merge though 🤞

Copy link

codecov bot commented May 13, 2024

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 81.94%. Comparing base (9b0b76b) to head (2e77ec0).
Report is 33 commits behind head on main.

❗ Current head 2e77ec0 differs from pull request most recent head 3ea81a4. Consider uploading reports for the commit 3ea81a4 to get more accurate results

Files Patch % Lines
mesmer/mesmer_m/power_transformer.py 0.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #444      +/-   ##
==========================================
- Coverage   87.90%   81.94%   -5.96%     
==========================================
  Files          40       44       +4     
  Lines        1745     1939     +194     
==========================================
+ Hits         1534     1589      +55     
- Misses        211      350     +139     
Flag Coverage Δ
unittests 81.94% <0.00%> (-5.96%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Member

@mathause mathause left a comment

Choose a reason for hiding this comment

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

Let's merge as is because this is definitely a plus. (However, that should eventually be rewritten (I don't like the "iterate over rows of the array approach" & this can maybe be vectorized).)

@mathause mathause changed the title Rename I and j in power transformer transform & inverse transform rename i and j in power transformer transform & inverse transform May 13, 2024
Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>
@veni-vidi-vici-dormivi veni-vidi-vici-dormivi merged commit ad839b7 into MESMER-group:main May 14, 2024
8 checks passed
@veni-vidi-vici-dormivi veni-vidi-vici-dormivi deleted the rename_i_j_pt branch May 14, 2024 07:07
veni-vidi-vici-dormivi added a commit to veni-vidi-vici-dormivi/mesmer that referenced this pull request May 23, 2024
…SMER-group#444)

* rename i and j to gridcell and year for readability and some commentary
---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>
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