Skip to content

Conversation

kelle
Copy link
Contributor

@kelle kelle commented Mar 25, 2025

fixes bug in ingest_publication when only an arxiv-id was provided. Adds relevant tests.

@dr-rodriguez
Copy link
Collaborator

Is there an associated ticket with the bug? Just wondering if there is an explicit example and any additional notes/information.

@kelle
Copy link
Contributor Author

kelle commented Apr 8, 2025

In addition to the arxix_id bug, I was also responding to Ruff letting me know the function was too complex.

@kelle kelle requested a review from dr-rodriguez April 8, 2025 21:26
)
return

if reference and not bibcode and not doi:
Copy link
Collaborator

Choose a reason for hiding this comment

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

In general, I recommend if statements be explicit unless they are booleans already. Neither reference, bibcode, or doi are booleans so you would want to use things like if reference is not None instead of just if reference

name_add = reference
using = f"ref: {reference} user input. No bibcode or DOI provided."

if ignore_ads is False and check_ads_token() is False:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, I would have done the opposite since these are booleans: if not ignore_ads and not check_ads_token()

Copy link
Collaborator

@dr-rodriguez dr-rodriguez left a comment

Choose a reason for hiding this comment

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

Approving this for now, but I think more could be done to clean this up.

@kelle kelle merged commit de9e91b into astrodbtoolkit:main Apr 11, 2025
3 checks passed
@kelle kelle deleted the fix-pubs branch April 11, 2025 18:02
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.

2 participants