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.

Sign up for GitHub

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

Jump to bottom

Update ids #363

Merged
merged 3 commits into from
May 29, 2024
Merged

Update ids #363

merged 3 commits into from
May 29, 2024

Conversation

jmelot
Copy link
Member

@jmelot jmelot commented May 24, 2024 •
edited
Loading

Use old PARAT ids if available, otherwise use new ones plus an offset to prevent overlap (4000 is larger than any old PARAT id)

Closes #284
Closes #295

@github-actions GitHub Actions
Copy link

github-actions bot commented May 24, 2024 •
edited
Loading

Needs rebasing ‼️
behind_count is 1
ahead_count is 3

@jmelot jmelot changed the base branch from master to 236-sector May 24, 2024 02:42
@github-actions GitHub Actions
Copy link

github-actions bot commented May 24, 2024 •
edited
Loading

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
580 408 70% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
web/scripts/retrieve_data.py 65% 🟢
TOTAL 65% 🟢

updated for commit: 8430209 by action🐍

@github-actions GitHub Actions
Copy link

github-actions bot commented May 24, 2024 •
edited
Loading

JavaScript Coverage

Summary

Lines Statements Branches Functions
Coverage: 66%
 66.89% (396/592) 55.68% (191/343) 67.74% (126/186)
Modified Files • (66%)
File% Stmts% Branch% Funcs% LinesUncovered Line #s
All files66.8955.6867.7466.6 
static_data65.5151.7277.2762.96 
   data.js100100100100 

Base automatically changed from 236-sector to master May 24, 2024 14:28
@jmelot jmelot requested a review from rggelles May 28, 2024 15:36
@jmelot jmelot marked this pull request as ready for review May 28, 2024 15:36
rggelles
rggelles approved these changes May 29, 2024
View reviewed changes
Copy link
Member

@rggelles rggelles left a comment

Choose a reason for hiding this comment

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

LGTM! Put some comments in mostly just trying to understand the changes made, but everything looks correct.

company_linkage/parat_scripts/aggregate_organizations.py
@@ -8,7 +8,7 @@
# note: check https://docs.google.com/spreadsheets/d/1Tq28O8qIA6T3AJ5oTHKCcscaNZsY_E4OPOUm6JaiwWA/edit#gid=0
# to ensure list is complete
# we might switch this to a query or something instead of hard-coding it but this is easier for now
no_roll_up = [101, 550, 1826, 313, 327, 2343]
no_roll_up = [550, 1826, 313, 327, 2343]
Copy link
Member

Choose a reason for hiding this comment

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

Is this a specific choice that we're now rolling up google into Alphabet?

company_linkage/sql/organizations.sql Show resolved Hide resolved
company_linkage/sql/organizations.sql Show resolved Hide resolved
web/scripts/retrieve_data.py Show resolved Hide resolved
@jmelot jmelot merged commit 3efff71 into master May 29, 2024
8 checks passed
@jmelot jmelot deleted the update-ids branch May 29, 2024 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rggelles rggelles rggelles approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add ability to override raw company names Google v Alphabet
2 participants
@jmelot @rggelles