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 floor patch for 0507.03.750 #561

Closed
wants to merge 1 commit into from
Closed

Conversation

octycs
Copy link
Contributor

@octycs octycs commented May 3, 2023

Resolves #560

(as PR so we can discuss)

Proposed Changes (include Screenshots if possible)

  • Patch the floor of this lecture hall to 2
  • Add an informational comment like for MW1801. If you consider it unnecessary we can remove the comment though

How to test this PR

  1. Open 0507.03.750 in a patched instance

How has this been tested?

Checklist:

  • I have updated the documentation / No need to update the documentation
  • I have run the linter

Resolves #560

(as PR so we can discuss)
@octycs octycs added the data This issue is related to the data collection and aggregation label May 3, 2023
@octycs octycs requested a review from CommanderStorm May 3, 2023 15:34
@octycs octycs self-assigned this May 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2023

👋 Thank you for contributing. A staging environment for this PR for this change will be available shortly

github-actions bot added a commit that referenced this pull request May 3, 2023
@CommanderStorm
Copy link
Member

I think that this is a more general issue (we don't handle that lecture halls normally span two floors)

Adding a comment for every one is imo too labour-intensive, given that we have about 130 Lecture halls (source: this is the number ProLehre quoted us for Livestreaming)

I dont know what is the best solution until we can detect this via the CAD maps from #403 (still ongoing, progress is slower than expected, sorry..)

@CommanderStorm
Copy link
Member

We can merge this or close the underlying issue (I am indifferent about which one we choose)

I think that the comment should be This {display_name} has entries on {floor1} and {floor2}. until we support rooms spanning floors...

@octycs
Copy link
Contributor Author

octycs commented May 8, 2023

I agree, lecture halls (or any other rooms) with entries in more than floor cannot be handled at the moment, and I'm also not aware of any way to detect that from our data (e.g. the interims I lecture halls also don't have a clear main entrance). Until there is a solution I would stick to using the main entrance if there is one and else whatever TUMonline uses, so we have fewer patches. I am not sure if this particular lecture hall has a clear main entrance

@CommanderStorm
Copy link
Member

It does not have a clear main entrance. Closing this and associated issue

@CommanderStorm CommanderStorm deleted the floor-patch-0507.03.750 branch May 8, 2023 20:51
github-actions bot added a commit that referenced this pull request May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data This issue is related to the data collection and aggregation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Entry] [0507.03.750]:
2 participants