Skip to content

Adding the programming language tag to TESTed#583

Open
BrentBlanckaert wants to merge 26 commits into
feat/add-natural-translationfrom
feat/add-programming-language-tag
Open

Adding the programming language tag to TESTed#583
BrentBlanckaert wants to merge 26 commits into
feat/add-natural-translationfrom
feat/add-programming-language-tag

Conversation

@BrentBlanckaert

@BrentBlanckaert BrentBlanckaert commented Mar 31, 2025

Copy link
Copy Markdown
Collaborator

This builds of the PR #574. The goal is to add the tag !programming_language such that it is more consistent with the usage of the tag !natural_language.

@BrentBlanckaert BrentBlanckaert marked this pull request as draft March 31, 2025 11:40
@niknetniko niknetniko changed the base branch from master to feat/add-natural-translation March 31, 2025 21:03
@BrentBlanckaert BrentBlanckaert self-assigned this Apr 8, 2025
@BrentBlanckaert BrentBlanckaert added enhancement dsl Affects the DSL. labels Apr 8, 2025
@BrentBlanckaert BrentBlanckaert marked this pull request as ready for review April 15, 2025 15:30

@jorg-vr jorg-vr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add tests:

  • old format still works but adds a deprecation warning
  • new format works
  • the deprecation warning is only added once

Comment thread tested/__main__.py Outdated
"--translate",
type=str,
help="Specifies the language to translate translate the dsl to.",
default="-",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What does this change do?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is actualy a change that already should be in the other branch. This default will cause the translator to run when the option -t isn't even provided.

Comment thread tested/dsl/dsl_errors.py Outdated
messages.append(
ExtendedMessage(
f"WARNING: You are using YAML syntax to specify statements or expressions in multiple programming languages without the `!programming_language` tag. This usage is deprecated!",
permission=Permission.STAFF,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@bmesuere, this gave me the idea that we might need to add a Permission.CREATOR or something like that in the future. Some messages just aren't very actionable for teachers using exercises that are not their own.

We might also want to mark exercises with unresolved warnings/errors in dodona and create a page to review these for content creators

Comment thread tested/dsl/dsl_errors.py Outdated
]


def build_translate_parser_messages(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wouldn't take a boolean argument here, nor would I return a list, as it is a single message
And rename it to build_deprecated_language_message or something like that

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I wrote it like this such that this function can easily be expanded when extra deprecation warnings pop-up about using file instead output_files.

Comment thread tested/dsl/dsl_errors.py Outdated
Comment thread tested/dsl/translate_parser.py Outdated
Comment thread tested/dsl/translate_parser.py Outdated
Comment thread tested/dsl/translate_parser.py Outdated
Comment thread tested/dsl/translate_parser.py Outdated
Comment thread tested/judge/core.py Outdated
Comment thread tested/judge/core.py Outdated
# TODO: In the PR of adding file usage to the DSL, other deprecation warnings were added during parsing.
# So I think it's a nice split to have messages from parser and from the preprocessor.
collector.add_messages(
build_translate_parser_messages(bundle.suite.using_deprecated_prog_languages)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as in the natural_languages pr, this method won't be needed here if we had bundle.parser_messages

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I've resolved all the other notes because it boils down to this. I changed the usage of booleans to an actual list of ExtendedMessages

@BrentBlanckaert BrentBlanckaert requested a review from jorg-vr April 29, 2025 16:22
Comment thread tested/main.py Outdated
Comment thread tests/test_dsl_yaml.py
Comment thread tested/dsl/translate_parser.py Outdated
Comment thread tested/dodona.py
@BrentBlanckaert BrentBlanckaert requested a review from jorg-vr May 9, 2025 15:08

@jorg-vr jorg-vr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, left some mostly minor nitpicks

Comment thread tested/descriptions/renderer.py Outdated
Comment thread tested/descriptions/renderer.py Outdated
Comment thread tested/utils.py Outdated
Comment thread tested/dsl/translate_parser.py
Comment thread tested/dsl/translate_parser.py Outdated
Comment thread tested/dsl/translate_parser.py Outdated
Comment thread tested/dsl/translate_parser.py Outdated
Comment thread tested/dsl/translate_parser.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dsl Affects the DSL.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants