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
Remi schema creator #6
Conversation
Merge pull request ReproNim#12 from sanuann/schema-creator
Remi schema creator
updates: - [github.com/pre-commit/pre-commit-hooks: v2.0.0 → v4.0.1](pre-commit/pre-commit-hooks@v2.0.0...v4.0.1) - [github.com/psf/black: 19.3b0 → 21.7b0](psf/black@19.3b0...21.7b0)
for more information, see https://pre-commit.ci
[pre-commit.ci] pre-commit autoupdate
updates: - [github.com/pre-commit/pre-commit-hooks: v4.0.1 → v4.1.0](pre-commit/pre-commit-hooks@v4.0.1...v4.1.0) - [github.com/psf/black: 21.7b0 → 22.1.0](psf/black@21.7b0...22.1.0)
[pre-commit.ci] pre-commit autoupdate
updates: - [github.com/pre-commit/pre-commit-hooks: v4.1.0 → v4.2.0](pre-commit/pre-commit-hooks@v4.1.0...v4.2.0) - [github.com/psf/black: 22.1.0 → 22.3.0](psf/black@22.1.0...22.3.0)
[pre-commit.ci] pre-commit autoupdate
add python 3.10 to matrix
updates: - [github.com/pre-commit/pre-commit-hooks: v4.2.0 → v4.3.0](pre-commit/pre-commit-hooks@v4.2.0...v4.3.0) - [github.com/psf/black: 22.3.0 → 22.6.0](psf/black@22.3.0...22.6.0)
…nto schema-creator
refactor run pre commit refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor context setting add validation refactor set_description refactor refactor naming items refactor naming items refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor
for more information, see https://pre-commit.ci
[pre-commit.ci] pre-commit autoupdate
refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor refactor test pollution test pollution test pollution test pollution test pollution test pollution test pollution test pollution test pollution test pollution test pollution test pollution test pollution test pollution test pollution test pollution refactor refactor refactor refactor refactor refactor refactor refactor add type hint refactor refactor refactor refactor
use new response option class rm old response option class
update doc
Hello @Remi-Gau! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-05-11 17:19:45 UTC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've reviewed this pull request using the Sourcery rules engine. If you would also like our AI-powered code review then let us know.
for i in range(1, 5): | ||
response_options.add_choice("", i) | ||
response_options.add_choice("Completely", 6) | ||
response_options.set_max(6) | ||
response_options.add_choice(value=i) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (code-quality): Avoid loops in tests. (no-loop-in-tests
)
Explanation
Avoid complex code, like loops, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
# -- Project information ----------------------------------------------------- | ||
|
||
project = "reproschema-py" | ||
copyright = "2022, Repronim developers" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (code-quality): Don't assign to builtin variable copyright
(avoid-builtin-shadow
)
Explanation
Python has a number ofbuiltin
variables: functions and constants thatform a part of the language, such as
list
, getattr
, and type
(See https://docs.python.org/3/library/functions.html).
It is valid, in the language, to re-bind such variables:
list = [1, 2, 3]
However, this is considered poor practice.
- It will confuse other developers.
- It will confuse syntax highlighters and linters.
- It means you can no longer use that builtin for its original purpose.
How can you solve this?
Rename the variable something more specific, such as integers
.
In a pinch, my_list
and similar names are colloquially-recognized
placeholders.
item.response_options.unitOptions = [unitOption_0.schema] | ||
item.response_options.unitOptions.append(unitOption_1.schema) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Merge append into list declaration (merge-list-append
)
item.response_options.unitOptions = [unitOption_0.schema] | |
item.response_options.unitOptions.append(unitOption_1.schema) | |
item.response_options.unitOptions = [unitOption_0.schema, unitOption_1.schema] |
No description provided.