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

Cli crashes when no name is provided #9

Closed
JoseKilo opened this issue Apr 23, 2023 · 4 comments · Fixed by #10
Closed

Cli crashes when no name is provided #9

JoseKilo opened this issue Apr 23, 2023 · 4 comments · Fixed by #10
Assignees
Labels
bug Something isn't working

Comments

@JoseKilo
Copy link

JoseKilo commented Apr 23, 2023

HI, nice tool ! Thanks for sharing it.

I've noticed a small bug while using the cli:
If I don't provide a chart name, it crashes without providing much info:

> candlestick-chart -m json-file -f candles.json 
Traceback (most recent call last):
  File "/home/jose/charts/.venv/bin/candlestick-chart", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/jose/charts/.venv/lib/python3.11/site-packages/candlestick_chart/__main__.py", line 59, in main
    chart.draw()
  File "/home/jose/charts/.venv/lib/python3.11/site-packages/candlestick_chart/chart.py", line 57, in draw
    print(self._render())
          ^^^^^^^^^^^^^^
  File "/home/jose/charts/.venv/lib/python3.11/site-packages/candlestick_chart/chart.py", line 53, in _render
    return self.renderer.render(self)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jose/charts/.venv/lib/python3.11/site-packages/candlestick_chart/chart_renderer.py", line 104, in render
    output.append(chart.info_bar.render(chart_data.main_candle_set, chart_data.width))
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jose/charts/.venv/lib/python3.11/site-packages/candlestick_chart/info_bar.py", line 60, in render
    " | ".join(
TypeError: object of type 'NoneType' has no len()
>

It works if a name is provided:

> candlestick-chart -m json-file -f candles.json --chart-name __NAME__

and even with an empty name:

> candlestick-chart -m json-file -f candles.json --chart-name ''

It looks like the default value for the argument should be a string, changing this line seems to fix it

parser.add_argument("--chart-name", help="Sets the chart name.")

    parser.add_argument("--chart-name", help="Sets the chart name.", default="")

I'm using the latest version from PyPI:

> candlestick-chart --version
candlestick-chart 2.6.0
@BoboTiG BoboTiG added the bug Something isn't working label Apr 23, 2023
@BoboTiG
Copy link
Owner

BoboTiG commented Apr 23, 2023

Thanks for the report @JoseKilo :)

Would you like to open a PR?

@BoboTiG BoboTiG self-assigned this Apr 23, 2023
@BoboTiG
Copy link
Owner

BoboTiG commented Apr 23, 2023

I've some free time, I will work on it, and add tests at the same time.

BoboTiG added a commit that referenced this issue Apr 23, 2023
Also:
- Fix Mypy error `PEP 484 prohibits implicit Optional`
- Use `shutil.get_terminal_size()` instead of `os.get_terminal_size()` to be able to run tests without hitting `OSError: [Errno 25] Inappropriate ioctl for device`
- Support for Python 3.12
- CI to run unit tests
- Tests for the CLI entry point
BoboTiG added a commit that referenced this issue Apr 23, 2023
- Fix Mypy error `PEP 484 prohibits implicit Optional`
- Use `shutil.get_terminal_size()` instead of `os.get_terminal_size()` to be able to run tests without hitting `OSError: [Errno 25] Inappropriate ioctl for device`
- Support for Python 3.12
- CI to run unit tests
- Tests for the CLI entry point
@BoboTiG
Copy link
Owner

BoboTiG commented Apr 23, 2023

v2.7.0 released with the fix 🎉

@JoseKilo
Copy link
Author

Amazing ! Thanks a lot !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants