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

Defaulting to Python 3 #15

Closed
jsannemo opened this issue Sep 6, 2022 · 17 comments
Closed

Defaulting to Python 3 #15

jsannemo opened this issue Sep 6, 2022 · 17 comments
Assignees
Labels
decided Has a decision.

Comments

@jsannemo
Copy link
Contributor

jsannemo commented Sep 6, 2022

Python 2 has now been EOL:ed for quite some time. Looking at e.g. NCPC 2020 Python 3 is >10x more popular than Python 2, and I expect this has only increased since.

I think it's time for the default Python version in the spec to be Python 3.

This is backwards-incompatible, but easily fixed for old problems: add a shebang on Python 2 code if the validator, submission etc crashes (typically they will have a print blah call that causes a syntax error).

Thoughts? e.g. @niemela @ghamerly @simonlindholm

@simonlindholm
Copy link
Member

Agreed. (And I'm usually very reluctant to make backward incompatible changes.)

Here's a script for automatically adding shebangs to Python 2 files that are missing them (or are using #!/usr/usr/env python to mean Python 2 which I don't know which behavior we want out of):

import sys

dry_run = False
files = []
for arg in sys.argv[1:]:
    if arg.startswith("-"):
        if arg == "--dry-run":
            dry_run = True
        else:
            print("bad arg:", arg)
            sys.exit(1)
    else:
        files.append(arg)

for filename in files:
    if not filename.endswith(".py"):
        continue
    with open(filename) as f:
        source = f.read()
    first_line = source.split("\n")[0]
    if "python2" in first_line or "python3" in first_line:
        continue
    if dry_run:
        print("would convert:", filename)
    else:
        print("converting:", filename)
        if first_line.startswith("#!/"):
            source = "\n".join(source.split("\n")[1:])
        source = "#!/usr/bin/env python2\n" + source
        with open(filename, "w") as f:
            f.write(source)

Usage: find -type f -name '*.py' -print0 | xargs -0 python3 add_python2_shebangs.py --dry-run

@niemela
Copy link
Member

niemela commented Sep 7, 2022

I think this would be a very good change.

Maybe we could introduce a "version" to the spec just before this. The idea being that version has to be increased for any non-backwards-compatible change. Then you could at least detect problems that are not using the latest version and in best case automatically translating them, works case manually doing so?

Thoughts?

On small but annoying issues is, what should the version field be called? I think that the obvious choice ("version") would be misinterpreted as the version of the problem.

@jsannemo
Copy link
Contributor Author

jsannemo commented Sep 7, 2022

The verbose problem_format_version maybe?

That would also help with the other backwards incompatible (but probably not in practice) change (with reject/accept score).

@deboer-tim
Copy link

Or spec_version to be shorter?

+1 to this idea, but worried that authors are not used to adding this attribute, and will be confused when (without it) things still default to python 2 years from now. Might be a good idea for tools to start warning if there's no spec/format version or something similar to hint them towards the new behaviour.

@RagnarGrootKoerkamp
Copy link
Contributor

Introducing a version/flag to enable the python3 default sounds a bit annoying long term to me.

Alternatively, we could have default_to_python2 (default false) in the config to get back the current behaviour.

@jsannemo
Copy link
Contributor Author

jsannemo commented Sep 7, 2022

I agree that py3 probably makes sense as the default even for non-versioned problems, but don't feel strongly on the matter. When fixing some old domjudge-run contests my impression was that they already do default to python3 (not compliant), so in practice things seem to be a bit up in the air anyway.

I don't like adding an extra flag for this: just adding a shebang is equivalent and cleaner than modifying the spec for such a special case IMO.

@niemela
Copy link
Member

niemela commented Sep 7, 2022

Introducing a version/flag to enable the python3 default sounds a bit annoying long term to me.

That is absolutely not the point. The version flag would basically be required, not just for python3.

The version flag is a good idea in itself, and us doing a non-backwards-compatible change is just a reminder that we really should do that.

@jsannemo
Copy link
Contributor Author

jsannemo commented Sep 7, 2022

@niemela I agree that's not the point, but also think that's not @RagnarGrootKoerkamp s point.

If I interpret him correctly, he's saying that for /practical/ matters it would be annoying if the implicit version V0 specified by the lack of the flag has python2 as default, and suggests that V0 should have py3 as default and the new flag for version should take effect /after/ this change.

I agree with that opinion.

@RagnarGrootKoerkamp
Copy link
Contributor

@jsannemo yes exactly. Having to set a flag somewhere only for the purpose of having a python3 default would be annoying.

@niemela
Copy link
Member

niemela commented Sep 7, 2022

Yeah, I think I would be fine with that. (Basically, we're considering Python 2 to be a bug, not Python 3 to be a feature 😄).

That said, if we introduce the version tag I would suggest treating version-less problem.yaml to be deprecated and cause at least warnings (eventually errors).

@niemela
Copy link
Member

niemela commented Sep 7, 2022

@jsannemo yes exactly. Having to set a flag somewhere only for the purpose of having a python3 default would be annoying.

Yes, agreed. But, again, that's not the point. At all. The version flag should always be set if we introduce it. The version-less would only be a "compatability mode".

@deboer-tim
Copy link

Another option is to look at it as 'lack of version' = 'tool gets to decide' and newer tools can default to spec v2 with python v3 as default. As Fredrik says, add a warning to push people towards specifying a version.

@jsannemo
Copy link
Contributor Author

jsannemo commented Sep 7, 2022

Cool, I think there's essentially a consensus here. I'll draft some language for a PR.

@niemela
Copy link
Member

niemela commented Sep 7, 2022

Lets move further discussions about version fields to #18 .

jsannemo added a commit to jsannemo/problem-package-format that referenced this issue Sep 7, 2022
As discussed in Kattis#15.

This PR does not include any wording on versioning of the spec or compatibility; that will come as part of Kattis#18 instead.
@evouga
Copy link
Contributor

evouga commented Jul 21, 2023

I want to be part of this group

@Tagl
Copy link
Contributor

Tagl commented Jul 21, 2023

Me too

@jsannemo jsannemo added the decided Has a decision. label Jul 22, 2023
@niemela
Copy link
Member

niemela commented Jul 22, 2023

Fixed by #57

@niemela niemela closed this as completed Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
decided Has a decision.
Projects
None yet
Development

No branches or pull requests

7 participants