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

Group options in patent trends chart's dropdown #387

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

brianlove
Copy link
Contributor

Organize the various options in the dropdown menu controlling the patent trends chart to group related options together and remove other (non-charted) options.

Closes #337, closes #357

Organize the various options in the dropdown menu controlling the
patent trends chart to group related options together and remove other
(non-charted) options.

Closes #337, closes #357
@brianlove brianlove requested a review from jmelot May 31, 2024 18:05
@brianlove brianlove self-assigned this May 31, 2024
Copy link

No need for rebasing 👍
behind_count is 0
ahead_count is 1

Copy link

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
594 423 71% 0% 🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: 78c516e by action🐍

Copy link

JavaScript Coverage

Summary

Lines Statements Branches Functions
Coverage: 67%
67.45% (398/590) 54.18% (207/382) 68.1% (126/185)
Modified Files • (67%)
File% Stmts% Branch% Funcs% LinesUncovered Line #s
All files67.4554.1868.167.2 
components64.1754.3662.5963.72 
   DetailViewPatents.jsx000015–216

Copy link
Member

@jmelot jmelot left a comment

Choose a reason for hiding this comment

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

Looks good! I will note that this does not fully resolve #357 - e.g. see Microsoft in Robotics, Document management and publishing, and Military. These are all fairly rare (e.g. Robotics only occurs in 28 companies, Document Management and Publishing occurs in 16, and Military occurs in 12) (fyi @za158)

Do you want to handle subfields without patents in the UI, or do you want me to filter these out on the backend? If the latter, just leave that issue open

@brianlove
Copy link
Contributor Author

I don't think the Microsoft in [Robotics, Document Management & Publishing, Military] cases you mentioned are consistent with #357. In that issue, the problem is that there are no underlying data (counts is an empty array), so the x-axis shows the interval [-1, 6] (likely a Plotly default). In these cases, however, the data are present (via the group's counts value in overall_data.json), they are just extremely low (mostly zeros for the y-axis, with one year reaching 1). The x-axis, however, for these cases still have the correct years displayed (2013-2023).

We may want to handle low-data categories like these differently, but I think that's an analytic question to resolve, not a technical issue with the current state of the app.

@jmelot
Copy link
Member

jmelot commented Jun 3, 2024

Ah ok got it, I misread that issue. I'll ping Zach about what to do about subfields without patent data

@jmelot jmelot merged commit ca5108a into master Jun 3, 2024
6 checks passed
@jmelot jmelot deleted the 337-group-patent-dropdown-entries branch June 3, 2024 20:16
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.

Multiple patents data fields with no trends data Rework patent dropdown in detail view
2 participants