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

more rebust copyright parsing #127

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

RoootTheFox
Copy link

This PR intends to fix #126, preventing lack of copyright information from the API.

I've made quite a few changes to copyright parsing, trying to not break parsing older APODs - i manually verified parsing for a few older ones, and it doesn't seem to break any.

It also cleans up the copyright line before returning it in the 2nd copyright parsing attempt, preventing double spaces and weird/unnecessary newlines (\n) and double/triple spaces ( , ).

it also checks for explicit NASA credit - if no mentions of "copyright" or "license" have been found in the image credits, and NASA is explicitly credited, it assumes the image to be Public Domain and skips returning copyright information.
i also thought about introducing a "credits" line, specifically for cases like this, where there's no specific license/copyright information present but you still want to attribute the source of the image properly.

feedback is very much appreciated!

side note: the code might not be the best, i don't usually write python, but i tried to document it well enough for others to be able to understand it :3

intends to fix nasa#126

Signed-off-by: rooot <hey@rooot.gay>
Signed-off-by: rooot <hey@rooot.gay>
@RoootTheFox
Copy link
Author

small side note: the way I got nosetest to install and work was by running pip install pynose
i then verified that all tests pass by running nosetests -v tests/apod/test_utility.py 2>&1 | grep -i assert, which is non-ideal but seemed to work correctly ^^

@RoootTheFox
Copy link
Author

another thing I noticed: some (recent?) images return extra text in the explanation field than they're supposed to, like today's APOD:

[...] Explore Your Universe: Random APOD Generator

been considering either making another PR for this, or just having this PR fix that too

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.

API response is missing credit/license for most images
1 participant