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

Update client with argparse (#3) #25

Merged
merged 5 commits into from
Oct 14, 2024
Merged

Conversation

BugliL
Copy link
Contributor

@BugliL BugliL commented Jun 14, 2024

Pull Request

Description

Rewrited part of the client using argparse.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

All instruction to test this code is written in the README.md file.
All tests are in the folder tests/

Test Configuration:

  • Firmware version:
  • Hardware:
  • Toolchain:
  • IGN Version:

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

@BugliL BugliL added the enhancement New feature or request label Jun 14, 2024
@BugliL BugliL self-assigned this Jun 14, 2024
@BugliL BugliL requested a review from Wabri as a code owner June 14, 2024 17:02
@TheJoin95
Copy link
Member

note: please update to the latest image-go-nord package version

@BugliL BugliL force-pushed the main branch 3 times, most recently from 94350ff to 1a4e4c8 Compare July 6, 2024 17:08
src/image_go_nord_client/configs/messages.py Show resolved Hide resolved
src/image_go_nord_client/main.py Outdated Show resolved Hide resolved
tests/README.md Outdated Show resolved Hide resolved
tests/unit/argparser/test_colors.py Show resolved Hide resolved
OUTPUT_IMAGE_NAME = "nord" + DEFAULT_EXTENSION

__doc__ = """ImageGoNord, a converter for a rgb images to norththeme palette.
Usage: gonord [OPTION]...
Copy link
Member

Choose a reason for hiding this comment

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

Maybe better:

Suggested change
Usage: gonord [OPTION]...
Usage: ign [OPTION]...

did we decided to reduce the command to something thin like ign in the end? Or is it still a point for discussion? I dont have a good memory and don't find where we had the discussion 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, but I think this is better in another PR

tests/unit/argparser/test_colors.py Show resolved Hide resolved
tests/unit/mocked/test_client_basic_behavior.py Outdated Show resolved Hide resolved
Test added: No avg pixel

Test added: No avg pixel

refactor: moved old tests

feat: added settings for vscode

refactor: moved tests

refactor: moving files

test: creating mocked tests

test: first mocked test

test: added base tests

test: full mock

feat: added wrong parameter

refactor: removed useless params

refactor: tests + main

refactor: introducing argparse

refactor: using sys.stderr for img and version

refactor: quiet mode

refactor: output file

refactor: no-avg

refactor: blur and logging

refactor: black

refactor: pixels-area

refactor: small fixes

refactor: small fixes

refactor: moving to __init__

refactor: pattern selection

refactor: version

refactor: updated readme

feat: updated readme

feat: updated readme
refactor(palette): tests

feat(palette): updated version
@BugliL BugliL requested a review from Wabri October 6, 2024 18:38
@Wabri Wabri mentioned this pull request Oct 7, 2024
Copy link
Member

@TheJoin95 TheJoin95 left a comment

Choose a reason for hiding this comment

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

Overall looking good, it's kind of a giant PR but I understand that there was no other choice. Please attach the log or screenshot of a successful run, then I believe we can merge @Wabri

@Wabri
Copy link
Member

Wabri commented Oct 8, 2024

I'll leave the honor to @BugliL for his great and huge work!

@BugliL
Copy link
Contributor Author

BugliL commented Oct 10, 2024

Please attach the log or screenshot of a successful run

What do you mean? Just a simple run with the result?

@TheJoin95
Copy link
Member

Exactly

@BugliL
Copy link
Contributor Author

BugliL commented Oct 11, 2024

profile
serenade
nord

> python src/image_go_nord_client -i ~/Pictures/Screenshots/profile.png --palette nord -o ./nord.png
> python src/image_go_nord_client -i ~/Pictures/Screenshots/profile.png --palette serenade -o ./serenade.png

@TheJoin95
Copy link
Member

You can merge

@Wabri Wabri merged commit 7e65d0b into Schroedinger-Hat:main Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants