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

fix protect all classes #713

Merged
merged 4 commits into from
Jun 7, 2024

Conversation

juan-g-bonilla
Copy link
Contributor

Description

The method was always affecting Basilisk.architecture.swig_common_model instead of whatever module was given. This corrects this. Moreover, it replaces the use of exec with normal code, which should be a lot faster.

Verification

Added src\tests\test_protectAllClasses.py. I expect the change to break code that depended on these classes not being actually protected.

@juan-g-bonilla juan-g-bonilla added the bug Something isn't working label Jun 6, 2024
@juan-g-bonilla juan-g-bonilla self-assigned this Jun 6, 2024
@juan-g-bonilla juan-g-bonilla requested a review from a team as a code owner June 6, 2024 05:02
@juan-g-bonilla juan-g-bonilla force-pushed the feature/bsk-712-fix-protectAllClasses branch from a514881 to 284bc70 Compare June 6, 2024 05:03
@juan-g-bonilla juan-g-bonilla force-pushed the feature/bsk-712-fix-protectAllClasses branch from 284bc70 to 43a6f36 Compare June 6, 2024 05:10
@schaubh schaubh self-requested a review June 6, 2024 10:44
@schaubh schaubh changed the title Feature/bsk 712 fix protect all classes fix protect all classes Jun 6, 2024
@schaubh schaubh self-requested a review June 6, 2024 19:13
Copy link
Contributor

@schaubh schaubh 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 the quick support!

@juan-g-bonilla juan-g-bonilla merged commit fdc00bf into develop Jun 7, 2024
3 checks passed
@juan-g-bonilla juan-g-bonilla deleted the feature/bsk-712-fix-protectAllClasses branch June 7, 2024 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

protectAllClasses does not actually prevent from setting new attributes
2 participants