-
Notifications
You must be signed in to change notification settings - Fork 36
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
Updates to Sheets places_in.md file #367
Conversation
donaldrgosselin
commented
Sep 27, 2023
•
edited
Loading
edited
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please verify that the document looks good locally.
Also curious why there is a thick black and green border on the images.
api/sheets/places_in.md
Outdated
|
||
**Formula**: `=DCPLACESIN(dcids, placeType)` | ||
## Formula {#formula} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please check the formatting of these shortcuts (or what's in the brackets). here and below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified
api/sheets/places_in.md
Outdated
|
||
## Assembling the information you will need to use this formula | ||
This section contains examples of using the `=DCPLACESIN(*dcids*, *placeType*)` formula to return lists of child places from a list of parent [Place](https://datacommons.org/browser/Place) [DCIDs](https://docs.datacommons.org/glossary.html), such as [State](https://datacommons.org/browser/State), [Country](https://datacommons.org/browser/Country), and so. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please verify formatting of the formula. Since it's within ``, the formatting within doesn't work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified.
api/sheets/places_in.md
Outdated
|
||
This endpoint also requires the argument `placeType`, specifying the type of the child places you desire in the response. | ||
![alt_text](../../assets/images/sheets/sheets_counties_delaware.png "image_tooltip") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please give a proper tooltip
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the tooltip since we're not using them elsewhere.
api/sheets/places_in.md
Outdated
|
||
Before trying this method out, make sure to follow the setup directions in [the main section for Sheets docs](/api/sheets/index.html). | ||
![alt_text](../../assets/images/sheets/sheets_districts_alaska_hawaii.png "image_tooltip") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please give a proper tooltip
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the tooltip since we're not using them elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also ensure that the function call is included in the screenshot (like it is currently). At this point I prefer the existing screenshots on https://docs.datacommons.org/api/sheets/places_in.html
|
||
This endpoint requires the argument `dcids`. [DCIDs](/glossary.html) are unique node identifiers defined by Data Commons. Your query will need to specify the DCIDs for the parent places of interest. | ||
### Example 1: Retrieve a List of Counties in Delaware |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add descriptions for these examples like the GETNAME page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bo said he would prefer creating videos for each example, although I do like the example descriptions in the GETNAME page. Please let me know what you decide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should still have text descriptions, even if there are videos or images.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please update filenames in place (rather than adding new filenames)
comments from other PR's apply here too, such as removing the green border around images
api/sheets/places_in.md
Outdated
@@ -1,58 +1,38 @@ | |||
--- | |||
layout: default | |||
title: Places within a Place | |||
title: Retrieving Child Nodes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function only works with place nodes. Please update the title here and below to reflect that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to "Retrieving Places Contained Within Another Place"
api/sheets/places_in.md
Outdated
nav_order: 2 | ||
parent: Google Sheets | ||
grand_parent: API | ||
--- | ||
|
||
# Retrieve places contained within another place | ||
# Retrieving Child Nodes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to "Retrieving Places Contained Within Another Place"
* Conformed file to get_name.md * Added Returns section * Added Error Returns section * Replaced images with narrower versions * Added steps to examples * Updated Gemfiles
api/sheets/places_in.md
Outdated
|
||
## Assembling the information you will need to use this formula | ||
Lists of child places from a list of parent [Place](https://datacommons.org/browser/Place) [DCIDs](https://docs.datacommons.org/glossary.html), such as [State](https://datacommons.org/browser/State), [Country](https://datacommons.org/browser/Country), and so on. It only returns children with a place type that matches the <code>placeType</code> parameter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rephrase this - State and Country are place types (not place dcid's). I think you want to say something like this: returns a list of child places of the specified place dcids, of the specified place type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it as you suggested.
api/sheets/places_in.md
Outdated
3. Retrieve the DCIDs for the congressional districts by enter `=DCPLACESIN(A1:A2, "CongressionalDistrict")` into cell C1. | ||
4. Finally, retrieve the names of the congressional districts by entering `=DCGETNAMES(C1:C3)` into cell D1. | ||
|
||
![alt_text](/assets/images/sheets/sheets_places_in_congressional_districts_ak_hi.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use a meaningful alt_text (here and below)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the updates. a few comments below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please cut the "fill place dcid's" sidebar from this image - it's confusing to see Alaska in there, but the cell geoId/0200 highlighted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please cut the "fill place dcid's" sidebar from this image - it's confusing to see Delaware in there, but the cell for Kent County highlighted.
api/sheets/places_in.md
Outdated
|
||
**Formula**: `=DCPLACESIN(dcids, placeType)` | ||
## Formula {#formula} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
## Formula {#formula} | |
## Formula |
api/sheets/places_in.md
Outdated
|
||
* `dcids`: A list of (parent) `Place` nodes, identified by their DCIDs. | ||
## Required Arguments {#required-arguments} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
## Required Arguments {#required-arguments} | |
## Required Arguments |
api/sheets/places_in.md
Outdated
|
||
### Example 1: Retrieve a list of the counties in Delaware. | ||
1. Place your cursor in the cell where you want to add the DCID for Delaware. In this case, cell A2. | ||
2. Enter the Delaware DCID of “geoId/10”. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In line 55, we use bold instead of quotes to show what the user should enter. I personally prefer the bold, but we should probably standardize across examples (and the other pages.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. it's been hard to standardize across the PR's, and i can do a sweep after the first versions are in. i'll update these to bold for now though
thanks for the review! updated the formatting as we discussed offline |