Add extracting all patent counts to pipeline; modify other code for extensibility to enable this

[rggelles]

Note: this affects/modifies the grants PR

Closes #125

[github-actions]

No need for rebasing 👍
behind_count is 0
ahead_count is 14

[github-actions]

JavaScript Coverage

Summary

Lines	Statements	Branches	Functions
	67.87% (374/551)	59.58% (174/292)	67.55% (127/188)

Modified Files • (67%)

File	% Stmts	% Branch	% Funcs	% Lines	Uncovered Line #s
All files	67.87	59.58	67.55	67.5
components	64.4	62.11	62.01	63.84
DetailViewPublications.jsx	0	0	0	0	15–143
static_data	70.96	44	76.66	67.85
data.js	100	100	100	100

[jmelot]

Thank you! This looks good to me - I did have one substantive question about the use of a test parameter where I didn't expect it, but otherwise these are largely fussy comments.

[jmelot]

@rggelles I've started integrating this stuff, but I actually don't see all_patents in patent_visualization_data - has the pipeline been run since you made these changes, or does that column still need to get integrated into the final output table (or am I just blind - if so, sorry!)

[rggelles]

@rggelles I've started integrating this stuff, but I actually don't see all_patents in patent_visualization_data - has the pipeline been run since you made these changes, or does that column still need to get integrated into the final output table (or am I just blind - if so, sorry!)

This is because I forgot a change I needed to make in the omit_by_rule query; added this in as well for the update I'm about to push. My bad.

[rggelles]

@jmelot I believe all fixes should be in now, including at least two substantive ones. Pipeline is rerunning, although even the substantive reviews shouldn't affect the code significantly. Would appreciate a second review of the changes made when it finishes. Thanks!

[rggelles]

(it does appear to be failing some of your tests, but I think those are related to the user interface so it's not clear to me what's going on here or what I need to fix).

[jmelot]

Reading over this, looks good, thanks! I'll have another try at integration soon (and you're right, the broken tests aren't related to anything you did - I got part of the way through integration and then stopped which left things in a broken state)

[jmelot]

Just checked this again - I think something is messed up: SELECT count(0) FROM ai_companies_visualization.patent_visualization_data where all_patents is not null = 721 but SELECT count(0) FROM ai_companies_visualization.patent_visualization_data where ai_patents is not null = 2478

[rggelles]

@jmelot Believe this should be fixed; I failed to recompile the docker container for the last run but it's done now.

[rggelles]

Just pinging on this again -- is there anything else that needs to be done to close this? This field should be available and usable now.

[brianlove]

With the merging of version2 into master (#163), I've re-targeted this PR at master.

[jmelot]

@rggelles I was (finally) trying to integrate this, but I noticed the data got much larger. It turns out that the number of rows in patent_visualization_data has drastically increased (compare number of rows in the last few snapshots in ai_companies_visualization_backups for example). I think this may be due to duplicate rows creeping in - for example, SELECT count(0) FROM ai_companies_visualization.patent_visualization_data where cset_id = 796 now returns 36 rows.

[rggelles]

@rggelles I was (finally) trying to integrate this, but I noticed the data got much larger. It turns out that the number of rows in patent_visualization_data has drastically increased (compare number of rows in the last few snapshots in ai_companies_visualization_backups for example). I think this may be due to duplicate rows creeping in - for example, SELECT count(0) FROM ai_companies_visualization.patent_visualization_data where cset_id = 796 now returns 36 rows.

Just pushed a fix for this; this was an issue in my aggregation in the SQL query rather than any problem in the actual data pulls. The pipeline is rerunning from that point (which is well after the expensive point so it shouldn't be an issue to run).

[jmelot]

Ok I think I've successfully integrated this. @brianlove can you please check my commits (7dff783, f6d1873, f3316b5, a2ca760) and then if all is good we can merge!

[brianlove]

👍