Skip to content

Updated get_constituent_citations#39

Open
FusionSandwich wants to merge 1 commit into
mainfrom
changes-to-get_constituent_citations
Open

Updated get_constituent_citations#39
FusionSandwich wants to merge 1 commit into
mainfrom
changes-to-get_constituent_citations

Conversation

@FusionSandwich
Copy link
Copy Markdown
Contributor

Corrected the spelling of consituent to constituent, added doc strings, and updated get_constituent_citations to separate citations with a ",".

Copy link
Copy Markdown
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

Nice catch - I've offered some options that avoid the successive append() approach. Sometimes it's necessary to explicitly append something in a loop, but here a python list comprehension may be a better choice.

Comment thread material-db-tools/material_db_tools.py
Copy link
Copy Markdown
Contributor

@Edgar-21 Edgar-21 left a comment

Choose a reason for hiding this comment

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

this lgtm @gonuke I think we should be good to merge?

@gonuke
Copy link
Copy Markdown
Member

gonuke commented Apr 5, 2025

this lgtm @gonuke I think we should be good to merge?

The comment I made above was marked as resolved and given a 👍 , but there as no subsequent commit to make the change.... it may not be worth blocking the merge, but I was expecting to see an update (which would have ended up i my queue and had this merged long ago)

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.

3 participants