-
Notifications
You must be signed in to change notification settings - Fork 8
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
WIP: #296 filter for PhD and post docs alongside advisors #23
base: recent_collaborators
Are you sure you want to change the base?
WIP: #296 filter for PhD and post docs alongside advisors #23
Conversation
- working on fixing cells with data validation
- reformatted the names into Last, First format - sorted the names by alphabetical order (by last name)
…it of PR to wrong branch
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 @SaniHarouna-Mayer . Please can you move over the comments from the other PR?
regolith/schemas.py
Outdated
@@ -1366,6 +1366,7 @@ | |||
"schema": { | |||
"type": "dict", | |||
"schema": { | |||
"advisor": {"required": False, "type": "string"}, |
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 think a description could help here.
Also, we will need an examplar (above). Add it to an existing education scopatz item.
Previous comments: Thanks @SaniHarouna-Mayer this looks very good, well done! How will it be used? we will need the code that calls it. Also, it will need a test. Finally, a thought is thatthink that we probably want to get phd and postdoc advisors only for a single person so we won't need to iterate over an input_contacts, though this is just a guess as I am not sure how you plan to deploy this.... We ill also need to dereference the advisor name, so if the advisor is sbillinge we will use fuzzy_retrieval to get this person either from the people or contacts collection. You may still be working on that. A helpful comment in the PR would help me know where you are with the PR. Thanks so much! Again, a fine piece of code, very clean!
sounds good. Basically in the main code it is just getting all the collaborators etc.
then we would generally load one more thing, the elements of the list S @SaniHarouna-Mayer btw this PR should be in to the recent-collaborators branch |
@sbillinge
Thank you! |
Hi @sbillinge, could you please doublecheck if I got the guideline the right way?
Right now there are no advisor entries at all in people and contacts. How should we proceed to get this updated in the end? More specifically, where can I find those information, since it won't be easily found googling like we did when updating the institutions and contacts? Thank you! |
Thanks for reaching out Sani. It looks as if you are doing Issue regro#296 which is to find PhD and post-doc advisors. If it was me, my plan might look like this:
|
Now, to move this forward more quickly I can put some discussion and thoughts I have already had. Let's work on (1) first, where to put the info. The obvious place is to put it into the |
Let's work on (2) . The way the program is run is |
for (3) it is a matter of copy-pasting code that is already there and adapting it. |
btw, my PhD advisors were Takeshi Egami and Peter Davies and my postdoc advisor was George Kwei. |
Thanks Simon, this is really helpful! |
(1):
I already implemented these changes quite similar to your ideas. I don't quite see the confusion with the advisor name. I changed it to mentor in education and employment. I'd like to stick with the same name in education and description since it describes the same relationship and the code would loose lucidity otherwise. |
13a36ad
to
3e0ebc5
Compare
7ee23f8
to
7983fdc
Compare
copy of recent PR to the right branch now..