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

Add image aspect ratio for Bluesky #844

Merged
merged 7 commits into from
Dec 12, 2024
Merged

Add image aspect ratio for Bluesky #844

merged 7 commits into from
Dec 12, 2024

Conversation

JoelOtter
Copy link
Contributor

Hey @snarfed, here's my attempt at adding the image aspect ratio. Please tear it apart with extreme prejudice, I haven't written Python for many months now.

I added Pillow as a dependency to Granary - do we need to add it to Bridgy too? It wouldn't work locally unless I installed it there but I probably am misunderstanding how the local installation stuff in Pip works.

Fixes #843

@JoelOtter
Copy link
Contributor Author

Oh this is going to annoy the tests isn't it. Can we use a real image URL for the test data?

@snarfed
Copy link
Owner

snarfed commented Dec 11, 2024

Hah, yes. I think what you'd do is add a small image file to the tests/testdata/ directory, then update test_create_with_media to read it in as binary an return its bytes in the mock requests.get response:

@patch('requests.post')
@patch('requests.get')
def test_create_with_media(self, mock_get, mock_post):
mock_get.return_value = requests_response(
'pic data', headers={'Content-Type': 'my/pic'})

@JoelOtter
Copy link
Contributor Author

That works, thanks!

Copy link
Owner

@snarfed snarfed left a comment

Choose a reason for hiding this comment

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

Test looks great! Thank you!

granary/bluesky.py Outdated Show resolved Hide resolved
granary/bluesky.py Outdated Show resolved Hide resolved
granary/tests/test_bluesky.py Outdated Show resolved Hide resolved
requirements.txt Show resolved Hide resolved
@JoelOtter JoelOtter requested a review from snarfed December 11, 2024 19:32
granary/bluesky.py Outdated Show resolved Hide resolved
@JoelOtter JoelOtter requested a review from snarfed December 11, 2024 21:16
granary/bluesky.py Outdated Show resolved Hide resolved
@snarfed
Copy link
Owner

snarfed commented Dec 11, 2024

Looking good. One last minor nit, and then add this to the changelog in the readme, and I think we're set!

@JoelOtter JoelOtter requested a review from snarfed December 11, 2024 23:05
@JoelOtter
Copy link
Contributor Author

Have added to README but please reword or move if it's wrong or in the wrong place :)

@snarfed
Copy link
Owner

snarfed commented Dec 11, 2024

Looks great! Thank you!

Are you stlil set up to run this locally? It'd be nice to try interactively and see if it actually works.

@JoelOtter
Copy link
Contributor Author

Yes I've already used it to post things :) before and after:

Screenshot 2024-12-10 at 18 57 38
Screenshot 2024-12-10 at 23 05 31

@snarfed
Copy link
Owner

snarfed commented Dec 12, 2024

Awesome! Thank you for testing! OK, merging now, really psyched for this, thanks again for the contribution.

@snarfed snarfed merged commit 7346709 into snarfed:main Dec 12, 2024
2 checks passed
snarfed added a commit to snarfed/bridgy that referenced this pull request Dec 12, 2024
snarfed added a commit to snarfed/bridgy-fed that referenced this pull request Dec 12, 2024
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.

Add aspect ratio to Bluesky image embeds
2 participants