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

Nested derived type bug fix #192

Merged

Conversation

rolfhm
Copy link
Contributor

@rolfhm rolfhm commented Nov 22, 2023

Possibly a solution to issue #182

@rolfhm rolfhm marked this pull request as draft November 22, 2023 15:02
Copy link

codecov bot commented Nov 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4f49f1a) 92.32% compared to head (6ffaa99) 92.33%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #192   +/-   ##
=======================================
  Coverage   92.32%   92.33%           
=======================================
  Files          95       95           
  Lines       17271    17272    +1     
=======================================
+ Hits        15946    15948    +2     
+ Misses       1325     1324    -1     
Flag Coverage Δ
lint_rules 96.22% <ø> (ø)
loki 92.32% <100.00%> (+<0.01%) ⬆️
transformations 91.47% <ø> (ø)

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.

@rolfhm rolfhm force-pushed the sbrm-substitute-expressions-parent-change branch from ba049e0 to 5231618 Compare November 24, 2023 11:20
@mlange05 mlange05 force-pushed the sbrm-substitute-expressions-parent-change branch from 8759dc8 to a0763b0 Compare January 31, 2024 08:42
@mlange05
Copy link
Collaborator

Hi @rolfhm I just found this, and have recently stumbled across a slightly better fix. I really like your test though, so I've just triggered a rebase, and will bring this in with the other fix on top, I think. Hope you don't mind?

@rolfhm
Copy link
Contributor Author

rolfhm commented Jan 31, 2024

No problem. Glad it was of some use.
Just noticed there's a double 'replace' in the test docstring

@mlange05 mlange05 marked this pull request as ready for review January 31, 2024 09:09
Copy link
Collaborator

@mlange05 mlange05 left a comment

Choose a reason for hiding this comment

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

Apologies for the delay in picking this up. The test is neat and the problem had been fixed in another branch already. I've brought this all up to speed with latest main and consolidated (probably should be a squash-merge). GTG from me.

@mlange05 mlange05 added the ready to merge This PR has been approved and is ready to be merged label Jan 31, 2024
Copy link
Collaborator

@reuterbal reuterbal left a comment

Choose a reason for hiding this comment

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

Thanks all, fix looks great and test is much appreciated. GTG!

@mlange05 mlange05 merged commit 8e5c155 into ecmwf-ifs:main Jan 31, 2024
8 of 12 checks passed
@rolfhm rolfhm deleted the sbrm-substitute-expressions-parent-change branch February 1, 2024 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge This PR has been approved and is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SCC-stack: UnboundLocalError when a field of a derived type is used to declare a temporary array
3 participants