Skip to content

Conversation

@jjotero
Copy link
Contributor

@jjotero jjotero commented May 27, 2021

This PR fixes a bug that was causing variables and parameters set as class attributes to be silently overridden. Bug reproducer:

class Base(rfm.RegressionTest):
    ...
    p = parameter([1])
    v = variable(int, value=1)

Base.p = [2] # This PR will disallow updating parameters like this.
Base.v = 2

@rfm.simple_test
class Derived(Base):
    def __init__(self):
        print(self.p) # Prints 1
        print(self.v) # Prints 1. With this PR this prints 2.

This PR disables setting/updating parameters when accessed as class attributes. This is because such parameter update would break the internal iterators on the parameter space. If this feature is required, explicit insert_parameter or update_parameter class methods could be added.

This might be needed by #563

@jjotero jjotero added this to the ReFrame Sprint 21.05.2 milestone May 27, 2021
@jjotero jjotero requested review from teojgo and vkarak May 27, 2021 09:54
@jjotero jjotero self-assigned this May 27, 2021
Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

Just a couple of minor comments.

@codecov-commenter
Copy link

codecov-commenter commented May 27, 2021

Codecov Report

Merging #1988 (e52babc) into master (6a00e3b) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1988      +/-   ##
==========================================
+ Coverage   87.59%   87.61%   +0.02%     
==========================================
  Files          50       50              
  Lines        8770     8788      +18     
==========================================
+ Hits         7682     7700      +18     
  Misses       1088     1088              
Impacted Files Coverage Δ
reframe/core/meta.py 100.00% <100.00%> (ø)
reframe/core/variables.py 96.03% <100.00%> (-0.02%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a00e3b...e52babc. Read the comment docs.

@vkarak vkarak merged commit 4f6e207 into reframe-hpc:master May 27, 2021
@jjotero jjotero deleted the bugfix/var-and-param-access branch May 29, 2021 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants