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 #235 allocatable classes #236

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

krystophny
Copy link
Contributor

@krystophny krystophny commented Nov 26, 2024

Fixes #235 . This PR sits on top of #230, #231, #232, #237, #238 and #239.

@krystophny krystophny marked this pull request as ready for review November 26, 2024 23:30
f90wrap/f90wrapgen.py Outdated Show resolved Hide resolved
f90wrap/f90wrapgen.py Outdated Show resolved Hide resolved
@krystophny krystophny mentioned this pull request Nov 27, 2024
f90wrap/f90wrapgen.py Outdated Show resolved Hide resolved
f90wrap/fortran.py Show resolved Hide resolved
f90wrap/f90wrapgen.py Show resolved Hide resolved
f90wrap/fortran.py Outdated Show resolved Hide resolved
@krystophny
Copy link
Contributor Author

@jameskermode this PR is ready for a full review and merge. @Rykath, @Xyhlon - please also have a look and comment if you see something odd.

The code now

  1. Wraps all objects that are used as a class anywhere into another wrapper type.
  2. Allows to access member variables of classes via this nested type structure.
  3. Adds an extra allocate in the auto-generated constructor for the inner object
  4. Skips explicit call to the final procedure in the destructor, as this is done automatically via deallocate. The previous behavior was most likely a bug.

The solution involves adding a used_as_class attribute. This requires traversing the parse tree another time in find_types, as the usage via class is defined in procedures after types are already known.

The change is significant, as the wrapper type is introduced also in cases that were previously handled with type alone even though class appeared, e.g. examples/arrays_in_derived_types_issue50. I believe the behavior is now more consistent, and all test cases pass after the change. Still, one may also test with some real-world downstream codes before a release.

@krystophny krystophny changed the title Issue235 allocatable classes Fix #235 allocatable classes Nov 28, 2024
@krystophny
Copy link
Contributor Author

@jameskermode could you already take a look at this PR? In the meantime we could finish also #233 to fix #41 on top of this.

PS: Tests are passing, but the "Build wheel" jobs are causing troubles and I don't understand why.

@jameskermode
Copy link
Owner

Thanks very much for this contribution. As you say it's a major chance so will need testing with some downstream codes wrapped with f90wrap to ensure there are no regressions not captured in the unit tests. I should have time to do that next week. Do I understand right that this PR should be merged before #233?

@krystophny
Copy link
Contributor Author

krystophny commented Dec 4, 2024

Sure! For now the main work is done and there is no hurry. You are right, #233 should be merged after this one. We will also be testing on our fork on https://github.com/itpplasma with some of our codes in the meantime. With #233, f90wrap will cover all the remaining requirements for interfacing modern code from a compiled language in Python with minimal effort (C++/pybind11 people already envy us now).

@krystophny
Copy link
Contributor Author

... or if you have time for a short videocall to discuss some points directly, just drop me an e-mail then.

@krystophny krystophny mentioned this pull request Dec 26, 2024
9 tasks
@krystophny krystophny force-pushed the issue235_allocatable_classes branch from f1d94a0 to d95e13d Compare December 26, 2024 15:56
@krystophny krystophny marked this pull request as draft December 26, 2024 16:03
@krystophny krystophny force-pushed the issue235_allocatable_classes branch from d95e13d to db919ac Compare December 26, 2024 17:27
@krystophny krystophny marked this pull request as ready for review December 26, 2024 17:28
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.

Support for allocatable polymorphic class objects
2 participants