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

Add bout_v6_coordinates_upgrader script #115

Merged
merged 10 commits into from
Oct 23, 2024

Conversation

tomc271
Copy link

@tomc271 tomc271 commented Oct 2, 2024

Add a python script that upgrades files to use the refactored Coordinates class.

@tomc271 tomc271 requested a review from ZedThree October 2, 2024 12:17
@dschwoerer
Copy link
Contributor

This seems to depend on boutproject/BOUT-dev#2873

Concerning boutproject/BOUT-dev#2873 (comment) - is the use of proxy classes still considered?
If so, it would be great if they could make it to v6 as well. Having tools to do the conversion is great, but if one wants to bisect, having libraries that are neither backwards nor forwards compatible is a huge pain.
I would really love if that could be done.

@ZedThree
Copy link
Member

ZedThree commented Oct 2, 2024

@dschwoerer It's related to those changes, but doesn't depend on them

Proxy classes would be nice, but they do also have significant downsides -- more difficult to debug, and tend to be fragile.

@dschwoerer
Copy link
Contributor

@dschwoerer It's related to those changes, but doesn't depend on them

@ZedThree maybe we mean different things by "depend on the PR" - i meant that users would only want to use this conversion script, iff the PR is applied to their BOUT++ version. Otherwise that certainly would break the code?

I think we could make v5 forward compatible by:

  • adding setters and getters
  • Allowing to call Field3D's with returning the field itself (with raising a warning)

Proxy classes would be nice, but they do also have significant downsides -- more difficult to debug, and tend to be fragile.

I misunderstood / misremembered your comment, and thought that would be intended to be part of #2873 - but now I think that is not planned?

@ZedThree ZedThree merged commit a241feb into boutproject:master Oct 23, 2024
4 checks passed
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.

4 participants