Conversation
sgfost
left a comment
There was a problem hiding this comment.
mostly structural stuff. LMK when the map is working and I'll take another look
frontend/vite.config.ts
Outdated
| }, | ||
| }, | ||
| optimizeDeps: { | ||
| exclude: ['jeep-sqlite/loader'] |
package-lock.json
Outdated
frontend/package.json
Outdated
| "style:fix": "prettier --write src/", | ||
| "tls": "npm run type-check && npm run test && npm run lint && (npm run style || npm run style:fix)" | ||
| "tls": "npm run type-check && npm run test && npm run lint && (npm run style || npm run style:fix)", | ||
| "dev": "vite --force" |
There was a problem hiding this comment.
package.json and package-lock.json conflict with the main branch. Probably easiest to revert the lockfile, rebase, and rebuild
There was a problem hiding this comment.
this is just for testing, right? I'd leave this untracked since it won't get used in production
frontend/src/assets/isocountries.py
Outdated
There was a problem hiding this comment.
I don't think this is used by anything, wouldn't belong in the client assets/ if it was
d0164e6 to
8db6723
Compare
- fix refeernce to institutionData - add proj4 typescript typedefs
- remove dead imports - delete_ror_affiliations seems unnecessary, added a --force option to update_ror_affiliations instead - return dict object for update instead of tuple values in update_ror_affiliations.lookup_ror_id - fix invalid test_metrics method call ref
- use requests.Session context manager instead of caching on the Command instance - add with_affiliations() queryset method to MemberProfile and other minor hygiene
pull a few more details from the ror api including wikidata ids for future semantic web support, acronyms, and aliases add ror api url to settings should probably make a ror api service at some point but seemed overkill and also the ror schema might benefit from codemeticulous changes in the github mirroring PR
There was a problem hiding this comment.
We should capture the additional metadata from RoR when we make new entries. This should be an easy fix and I can do it sometime in the next few hrs along with (hopefully) finding a better layout for the metrics page
Everything else looks good. Thanks @CharlesSheelam and @alee for fixing.
main ror api client remains on the client-side. The mgmt commands dealing with the ror api on the server are meant as one-offs so I'm not sure if there is a benefit to moving this to a wrapper api on the server * move to ROR api v2 * slightly adjusted what is stored for affiliations (the whole primary location object from ror instead of picking out fields)
alee
left a comment
There was a problem hiding this comment.
thanks @sgfost & @CharlesSheelam ! deployed on staging and lgtm
| data = response.json() | ||
| print(".", end="", flush=True) | ||
| return { | ||
| "name": data["name"], |
There was a problem hiding this comment.
I wonder if there's other data we should be storing from the ROR API while we're running this lookup:
- acronyms
- labels
- wikipedia_url
- wikidata
- country
also we should start using the ROR v2 API https://ror.readme.io/v2/docs/rest-api though this will change some of the fields that are getting pulled
https://ror.readme.io/v2/docs/schema-v2#forthcoming-v20-example-1

-seperated codebase and codebase release total counts on the metrics table
-set up the backend to gather institution data for the user map