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

Add type annotations to c_source.py #639

Merged
merged 8 commits into from
Jan 10, 2024

Conversation

zariiii9003
Copy link
Collaborator

  • add type annotations to c_source.py
  • use float literals if --use-float is set
  • do not cast double to float

Closes #604

@coveralls
Copy link

coveralls commented Dec 30, 2023

Pull Request Test Coverage Report for Build 7398162148

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 93.535%

Totals Coverage Status
Change from base Build 7359660138: -0.05%
Covered Lines: 7249
Relevant Lines: 7750

💛 - Coveralls

Copy link
Member

@andlaus andlaus left a comment

Choose a reason for hiding this comment

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

looks good

from .message import DecodeError as DecodeError
from .message import EncodeError as EncodeError
from .message import Message as Message
from .node import Node as Node
Copy link
Member

Choose a reason for hiding this comment

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

why the as {Foo}?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I checked csource.py with mypy --strict and it complained about that file. These names are exported according to PEP 484:

Modules and variables imported into the stub are not considered exported from the stub unless the import uses the import ... as ... form or the equivalent from ... import ... as ... form. (UPDATE: To clarify, the intention here is that only names imported using the form X as X will be exported, i.e. the name before and after as must be the same.)

Alternatively, we could put them in __all__ if you like.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. I just think that this is a quite clumsy construct. does mypy complain without --strict?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I replaced it with __all__. That works too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now i removed __all__, because it causes issues with these star imports:

# Remove once less users are using the old package structure.
from .can import *  # noqa: F403

src/cantools/database/can/c_source.py Outdated Show resolved Hide resolved
src/cantools/database/can/c_source.py Show resolved Hide resolved
src/cantools/database/can/c_source.py Outdated Show resolved Hide resolved
src/cantools/database/can/c_source.py Outdated Show resolved Hide resolved
src/cantools/database/can/c_source.py Outdated Show resolved Hide resolved
@zariiii9003 zariiii9003 merged commit f35551a into cantools:master Jan 10, 2024
16 checks passed
@zariiii9003 zariiii9003 deleted the float_literals branch January 10, 2024 19:35
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.

generate_c_source: --use-float should generate float constants
3 participants