Skip to content

Bugfix/waytype - #25

Open
andrejburja wants to merge 3 commits into
Tsadoq:mainfrom
andrejburja:bugfix/waytype
Open

Bugfix/waytype#25
andrejburja wants to merge 3 commits into
Tsadoq:mainfrom
andrejburja:bugfix/waytype

Conversation

@andrejburja

Copy link
Copy Markdown

amount in way type information can be equal to 1, not just less than 1

@Tsadoq
Tsadoq requested review from Tsadoq and Copilot April 2, 2025 15:09

Copilot AI 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.

Pull Request Overview

This PR fixes the validation for the "amount" parameter in way type information to allow values equal to 1.

  • Updated the amount comparison in the production code to allow an amount of 1.
  • Modified tests to no longer assert a failure for an amount equal to 1.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/test_waytype.py Adjusted test cases to remove the invalid boundary case for amount 1.
kompy/way_type.py Updated parameter documentation and validation to include 1 as a valid value.
Comments suppressed due to low confidence (1)

tests/test_waytype.py:32

  • Consider adding a test case to explicitly verify that initializing a WayType with an amount of 1 succeeds, ensuring proper boundary condition handling.
for amount in [-1, 0, 2]:

@Tsadoq

Tsadoq commented Apr 2, 2025

Copy link
Copy Markdown
Owner

@andrejburja hi! could you please bump up the version?

@Tsadoq
Tsadoq requested a review from Copilot April 2, 2025 15:16

Copilot AI 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.

Pull Request Overview

This PR fixes a bug in the way type validity checking by allowing an amount of 1, which was previously considered invalid.

  • Updated the initialization validation in kompy/way_type.py to permit an amount equal to 1.
  • Adjusted the tests in tests/test_waytype.py to remove the invalid test case for amount 1.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/test_waytype.py Removed test case for amount 1 to align with the new allowed boundary.
kompy/way_type.py Updated parameter documentation and condition check for the amount boundary.
Comments suppressed due to low confidence (1)

tests/test_waytype.py:29

  • [nitpick] The test docstring still indicates that an amount of 1 might be considered invalid, which is now allowed. Consider updating it to reflect that valid amounts are (0, 1].
Test initialization with invalid amounts (less than 0, equal to 0, greater than 1)

@Tsadoq
Tsadoq requested a review from Copilot April 2, 2025 15:26

Copilot AI 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.

Pull Request Overview

This PR addresses the bugfix for way type initialization so that an amount of 1 is now considered valid. Key changes include:

  • Updating the validation check in kompy/way_type.py to allow an amount equal to 1.
  • Adjusting the related error message in kompy/way_type.py.
  • Modifying the corresponding test in tests/test_waytype.py by removing 1 from the list of invalid amounts.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/test_waytype.py Removed 1 from invalid amounts to reflect the new valid condition.
kompy/way_type.py Changed the validation and error message to allow amount equal to 1.
Comments suppressed due to low confidence (1)

tests/test_waytype.py:31

  • Consider adding a test case to verify that initializing with an amount equal to 1 does not raise a ValueError, ensuring the bugfix behaves as expected.
valid_way_type = PossibleWayType.list_all()[0]

@Tsadoq Tsadoq left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@andrejburja I need you tu bumb up the version to be able to merge it

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.

4 participants