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 bahnv2 bike parking locked and open #172

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AbdullahiFatola
Copy link
Collaborator

This PR updates existing bahnv2 parking source converter by integrating the new capacity types BIKE_PARKING_LOCKED and BIKE_PARKING_OPEN as separate ParkingSite with type of LOCKBOX and OTHER respectively for bike parking. The existing mappings for car parking type are not changed.

The parking sites from Deutsche BahnPark bahnv2 now have purpose as bike and car , grouped into ParkingSiteGroup.

@@ -70,3 +60,39 @@ def get_data(self) -> dict:
timeout=60,
)
return response.json()


class BahnV2PullConverter(BahnV2BasePullConverter):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why creating three different converters for the same source? You can collect different purpose in the same source. This would have the advantage of not doing the same requests three times and therefore to prevent rate limits, and to have a simpler config and data handling.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right with the rate limit!

@@ -26,6 +33,13 @@ def map_static_parking_site(bahn_input: BahnParkingSiteInput) -> StaticParkingSi
has_realtime_data=False, # TODO: change this as soon as Bahn offers proper rate limits
static_data_updated_at=datetime.now(tz=timezone.utc),
public_url=bahn_input.url,
purpose=next(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why looping here at all? I mean, you select the type BahnParkingSiteCapacityType.PARKING anyway?

@@ -44,3 +58,93 @@ def map_static_parking_site(bahn_input: BahnParkingSiteInput) -> StaticParkingSi
static_parking_site_input.capacity_disabled = int(round(capacity_data.total))

return static_parking_site_input


class BahnBikeParkingLockedMapper:
Copy link
Contributor

Choose a reason for hiding this comment

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

That and the mapper below is almost a 100 % copy of the mapper above. In general, please try to avoid copy-pasting large parts of the code and do OOP. In this case, I wonder why you need different Mappers at all, as this could all be done in one source and one mapper.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's now updated.



@validataclass
class BahnBikeLockedParkingSiteInput:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as Mappers: copy paste, and can be removed completely as a single source would work, 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.

Updated, thanks!

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.

2 participants