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.

Sign up for GitHub

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

Jump to bottom

11 add domain specific data types for ship situation etc using pydantic models #12

Merged
merged 20 commits into from
Nov 27, 2023
Merged

11 add domain specific data types for ship situation etc using pydantic models #12

merged 20 commits into from
Nov 27, 2023

Conversation

ClaasRostock
Copy link
Contributor

Added domain specific data types for Ship, Situation etc. using pydantic models.
Changed all code to use the new data types.
Added type hints.
Refactored tests to work with the new data types.
Added tests for reading files and writing files.

@ClaasRostock
Added domain specific data types for Ship, Situation etc. using pydan…
fba9f06 
…tic models
@ClaasRostock
Changed all code to use the new data types. Added type hints.
185418a
@ClaasRostock
Sphinx documentation: resolved one issue raised by pyright in conf.py
2a9dc0b
@ClaasRostock
refactored tests to work with the new data types
3ed5fec
@ClaasRostock
added tests for reading files
fa5dc40
@ClaasRostock
added test for writing files
3e50900
@ClaasRostock
types.py: Changed enum classes to subclass from Enum instead of StrEn…
e7325a1 
…um, as StrEnum was introduced only in py311, but we want to support >= py39
@ClaasRostock
types.py: Changed declaration of optional fields
8d12f24
@ClaasRostock
types.py: Changed declaration of optional fields
c6ba720
@ClaasRostock
Changed declaration of optional fields
23d96c3
@ClaasRostock
test_read_files.py : Improved tests to be more robust to random chang…
5bca51a 
…es of reading sequence on GitHub runners.
@ClaasRostock
test_write_files.py : Improved tests to be more robust to random chan…
95137d6 
…ges of reading sequence on GitHub runners.
@ClaasRostock ClaasRostock linked an issue Nov 25, 2023 that may be closed by this pull request
Add domain specific data types for Ship, Situation etc. using pydantic models #11
Closed
@ClaasRostock ClaasRostock self-assigned this Nov 25, 2023
@ClaasRostock
test_trafficgen.py: test_gen_situations_1_ts_partly_spec_cli()
db91fb5 
Tweaked the test to avoid that this test randomly fails when run on GitHub.
@ClaasRostock ClaasRostock marked this pull request as draft November 25, 2023 13:28
@ClaasRostock
Copy link
Contributor Author

Hi @tomarnepedersen

I am currently comparing the output files from the tests between commit
Commit e8c2bba (open)
by Claas, 2023-11-23 14:44
and latest commit in this branch
Commit db91fb5 (open)
by Claas, 2023-11-25 01:24

I see there are some small differences in the output files.
Have hence reset this pull request to draft, until I investigated and we are sure that the changes in code do not change the output files (at least not content / number-wise).

@ClaasRostock
write_traffic_situation_to_file.py: exclude fields the value of which…
edd9e7c 
… is either None or the default value.
@ClaasRostock
write_traffic_situation_to_file.py: exclude fields the value of which…
e0e7793 
… is either None or has not been actively set after instantiation.
@ClaasRostock
Commit 185418a (open)
7483674 
by Claas, 2023-11-25 00:16

ship_traffic_generator.generate_traffic_situations() : Corrected an error I introduced with commit 185418a on 2023-11-25 00:16
lat_lon_0 of the new traffic_situation needs to be set to lat_lon_0 from the encounter settings. It was, though, set to lat_lon_0 from the desired traffic situation (which is usually None). I introduced the error when changing the code to use the new pdantic data types.
Now corrected. Also added a related assertion to the tests to ensure the error does not get introduced again.
@ClaasRostock
test_trafficgen.py : removed one ruff exemption
34368fe
@ClaasRostock
Copy link
Contributor Author

ship_traffic_generator.generate_traffic_situations() : Corrected an error I introduced with commit 185418a on 2023-11-25 00:16
lat_lon_0 of the new traffic_situation needs to be set to lat_lon_0 from the encounter settings. It was, though, set to lat_lon_0 from the desired traffic situation (which is usually None). I introduced the error when changing the code to use the new pdantic data types.
Now corrected. Also added a related assertion to the tests to ensure the error does not get introduced again.

@ClaasRostock ClaasRostock marked this pull request as ready for review November 25, 2023 16:01
@ClaasRostock
test_trafficgen.py: test_gen_situations_1_ts_full_spec_cli()
99df3c6 
Tweaked the test to avoid that this test randomly fails when run on GitHub.
@ClaasRostock
Copy link
Contributor Author

Hi @tomarnepedersen ,

this PR contains quite a lot of code changes as I introduced pydantic data classes to serve as domain specific types.
Please have a look whether you feel comfortable with it.

I also introduced type hints, resolving the 177 issues that pyright raised after activating the "reportMissingParameterType" rule in pyproject.toml.

There is one spot in the code where I suspect there might be an error. At least the docstring does not match the return values.
Would be good if you can throw a dedicated glimpse on that.
It is in module encounter.py, function calculate_relative_bearing().
If you search for @TODO in the code, you will easily find it.

Best greetings
Claas

@tomarnepedersen
Copy link
Collaborator

Done a few changes. Performing the merge.

@tomarnepedersen tomarnepedersen merged commit 544ddef into main Nov 27, 2023
10 checks passed
@tomarnepedersen tomarnepedersen deleted the 11-add-domain-specific-data-types-for-ship-situation-etc-using-pydantic-models branch November 27, 2023 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomarnepedersen tomarnepedersen Awaiting requested review from tomarnepedersen

Assignees

@ClaasRostock ClaasRostock

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add domain specific data types for Ship, Situation etc. using pydantic models
2 participants
@ClaasRostock @tomarnepedersen