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

Introduce building parts #51

Merged
merged 13 commits into from
Nov 16, 2023
Merged

Introduce building parts #51

merged 13 commits into from
Nov 16, 2023

Conversation

jwass
Copy link
Collaborator

@jwass jwass commented Sep 18, 2023

Introduce the building part schema. Parts and main footprints share shape information.

@jwass
Copy link
Collaborator Author

jwass commented Sep 18, 2023

Still todo:

  • More/better examples
  • More comprehensive docs
  • Is the building reference from part to building a string or something else?
  • Should the hasParts boolean be on the footprint if it can be determined by inspecting the data?

@jenningsanderson
Copy link
Collaborator

View the change in the documentation: https://dfhx9f55j8eg5.cloudfront.net/pr/51/reference/buildings/building

@jwass jwass requested a review from vcschapp September 20, 2023 15:20
@skmoore
Copy link
Contributor

skmoore commented Sep 20, 2023

@jwass Looking at the PR, I can't quite tell where the building ID that the part belongs to should go. Is it just an attribute underneath "properties" like this?

"properties": {
		"building": "1234",
		"extFoo": "I am a customer user property.",
		"extBar": "Me too!",
		"theme": "buildings",
		"type": "part",
		"version": 1,
		"level": 1,
		"updateTime": "2023-06-06T10:30:00-08:00",
		"numFloors": 8,
		"sources": [{
			"property": "",
			"dataset": "OpenStreetMap"
		}]
	}

@seanmcilroy29
Copy link
Contributor

@jwass - To update PR with the buildings ID, in line with @skmoore comments

@skmoore
Copy link
Contributor

skmoore commented Oct 2, 2023

@jwass Is the PR updated to show where the building ID value should go? Not sure if I missed it somewhere.

@jwass
Copy link
Collaborator Author

jwass commented Oct 3, 2023

@jwass Is the PR updated to show where the building ID value should go? Not sure if I missed it somewhere.

@skmoore Yes I just made the change in 051d72f. Good catch - I made it so the building id is now a required field and the example now contains the building field

@skmoore
Copy link
Contributor

skmoore commented Oct 3, 2023

@jwass How should I submit the sample data I put together?

@jwass
Copy link
Collaborator Author

jwass commented Oct 4, 2023

@jwass How should I submit the sample data I put together?

@skmoore Maybe make a new directory for each one under the examples/buildings directory in the repo?

@skmoore
Copy link
Contributor

skmoore commented Oct 5, 2023

@jwass Should I make a new PR or add to this one?

jwass and others added 7 commits October 17, 2023 11:44
Add the notion of a building part. Both building footprints and building
parts share in some shape definitions. These are pulled out in a common
defs.yaml file which are shared by buildings (footprints) and parts.

Validated/tested with the test.sh script
Was missing the building id in the part example
"sources": [{
"property": "",
"dataset": "OpenStreetMap"
}]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add hasParts set to True

@jwass jwass changed the title [WIP] Not ready to merge: Introduce building parts Not ready to merge: Introduce building parts Oct 31, 2023
@jwass jwass changed the title Not ready to merge: Introduce building parts Introduce building parts Oct 31, 2023
@jwass
Copy link
Collaborator Author

jwass commented Oct 31, 2023

Outstanding issue: https://github.com/OvertureMaps/tf-buildings/issues/24 before merging

@LukaszMichalski-TomTom
Copy link
Contributor

Should we include roof:material (link) and building:material(link) from OSM Simple 3D Buildings?

@jwass
Copy link
Collaborator Author

jwass commented Oct 31, 2023

Should we include roof:material (link) and building:material(link) from OSM Simple 3D Buildings?

@LukaszMichalski-TomTom I think we agreed at the meeting this is a good idea. Want to take a stab at adding it here?

@LukaszMichalski-TomTom
Copy link
Contributor

@jwass I don't have permission to commit to this repo. I prepared the change, but I cannot push it.

@jwass
Copy link
Collaborator Author

jwass commented Nov 13, 2023

@jwass I don't have permission to commit to this repo. I prepared the change, but I cannot push it.

Can you make a pull request to this branch? Or can't even do that?

@jwass
Copy link
Collaborator Author

jwass commented Nov 13, 2023

@jwass I don't have permission to commit to this repo. I prepared the change, but I cannot push it.

Can you try again? You should have permissions now

facadeMaterial, roofMaterial and facadeColor properties
@RobSoetewey-TomTom RobSoetewey-TomTom merged commit 192f07a into dev Nov 16, 2023
2 checks passed
@RobSoetewey-TomTom RobSoetewey-TomTom deleted the building_parts branch November 16, 2023 08: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.

8 participants