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

Remove goto #74

Closed
certik opened this issue Jun 6, 2022 · 7 comments
Closed

Remove goto #74

certik opened this issue Jun 6, 2022 · 7 comments

Comments

@certik
Copy link
Member

certik commented Jun 6, 2022

The code has about 30 occurrences of goto. We should replace it with modern Fortran's exit, cycle, return and other flow constructs.

This is very important to make people more excited to use the new code base (such as for SciPy #14), see e.g. this comment:

I’m not sure if that’s what actually modern Fortran looks like - I had expected not, because there’s still a lot of goto's in the code (which is bad).

@milancurcic
Copy link
Member

FWIW, the use of goto in the current minpack is fine. Most of them are to avoid repeating the error handling boilerplate. I would otherwise recommend not changing it, but I understand that from a cultural perspective it's important to change it. If it's important for SciPy for goto to go, let's do it.

@jacobwilliams
Copy link
Member

jacobwilliams commented Jun 6, 2022

GOTOs gotta go. Perception is very important. We can refactor and not duplicate code.

@certik
Copy link
Member Author

certik commented Jun 6, 2022

Perception is everything for minpack, it's our "poster child". Let's figure out how to do the error handling correctly without goto, and without losing any performance and without duplicating code.

@rgommers
Copy link

rgommers commented Jun 6, 2022

FWIW, the comment @certik linked was mine. Please feel free to take anything I say with a few grains of salt, since I really don't know much about Fortran. C code has some goto's sometimes as well, but they're usually goto error or goto exit; to me that is very different from goto 300, since I don't have to read the lines of code to figure out what the purpose of the goto is.

@certik
Copy link
Member Author

certik commented Jun 6, 2022

@rgommers actually, what you say is very important, as you are our ideal "customer". You are not a Fortran person, but you are in charge of quite a lot of Fortran code in SciPy and you are not against Fortran if we can fix all the very real technical problems with Fortran (in the broader sense). So we really appreciate your frank feedback.

@arjenmarkus
Copy link
Member

arjenmarkus commented Jun 6, 2022 via email

jacobwilliams added a commit to jacobwilliams/minpack that referenced this issue Jun 7, 2022
@jacobwilliams
Copy link
Member

See the PR #75. So far, just for hybrd, but it's using this approach. added a block and two do loops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants