Skip to content

Rewrite fypp directive to avoid the use of removesuffix in stdlib_math_meshgrip #847

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

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

jvdp1
Copy link
Member

@jvdp1 jvdp1 commented Jul 7, 2024

removesuffix used in stdlib_math_meshgrid requires Python >3.9.

Here is a small change to avoid the requirement of Python >3.9.

@igirault : was there a special need to use removesuffix? Do I overlook something? Or are my changes satisfying?

\cc @perazz @jalvesz

@jvdp1 jvdp1 requested review from perazz and a team July 7, 2024 13:06
Copy link
Member

@perazz perazz left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @jvdp1. Besides the greater compatibility, I also like your syntax better.

@jalvesz
Copy link
Contributor

jalvesz commented Jul 7, 2024

LGTM @jvdp1 indeed it looks cleaner this way!!

@perazz
Copy link
Member

perazz commented Jul 8, 2024

@jvdp1 I think this can be merged.

@igirault
Copy link
Contributor

igirault commented Jul 8, 2024

@igirault : was there a special need to use removesuffix? Do I overlook something? Or are my changes satisfying?

not at all, this was due to my poor understanding of the join operator in python. This is indeed way nicer with your correction :)

@jvdp1
Copy link
Member Author

jvdp1 commented Jul 8, 2024

Thank you @igirault for your answer.

With all these approvals, I will merge this PR.

@jvdp1 jvdp1 merged commit 70d34a4 into fortran-lang:master Jul 8, 2024
17 checks passed
@jvdp1 jvdp1 deleted the meshgrip_fypp branch July 8, 2024 07:57
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