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 for swig 4.1.X #159

Merged
merged 1 commit into from
Dec 16, 2022
Merged

Fix for swig 4.1.X #159

merged 1 commit into from
Dec 16, 2022

Conversation

chrispap95
Copy link
Collaborator

Swig 4.1.X changed the way it handles functions with @staticmethod. Up to version 4.0.2 it used to produce an additional flat-named function for each static method. For example, the function ClusterSequence.fastjet_banner_stream() used to also have a flat-name counterpart: ClusterSequence_fastjet_banner_stream(). Now, this is gone.

More info here swig/swig#2137

I just removed those imports. Tests seem to be fine and I couldn't find any uses for them.

@jmduarte jmduarte self-requested a review December 16, 2022 14:26
Copy link
Collaborator

@jmduarte jmduarte left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@jmduarte jmduarte merged commit 0b4351d into main Dec 16, 2022
@jmduarte jmduarte deleted the swig-4.1.x branch December 16, 2022 14:26
@lgray
Copy link
Collaborator

lgray commented Dec 16, 2022

Given one of these is some terminal banner for fastjet, check with the authors they don't want that appearing when you load the module or something. Even if it's not in the license it may be a good idea to check? Otherwise these look pretty benign, indeed.

@chrispap95
Copy link
Collaborator Author

@lgray I admit I didn't check whether the banner still shows up or not. However, my guess is that it is unrelated to these static methods. I will check and I will reply back here.

@chrispap95
Copy link
Collaborator Author

Just verified. Banner shows up as expected.

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.

3 participants