Kosmorro / kosmorro

A program to calculate your ephemerides.
https://kosmorro.space
GNU Affero General Public License v3.0
60 stars 7 forks source link

Guess the output format based on the --output argument #160

Closed Deuchnord closed 3 years ago

Deuchnord commented 3 years ago

(Feature request by @AmauryCarrade on Discord)

Is your feature request related to a problem? Please describe. Currently, to export in a PDF file, we need to give two arguments to the command:

Describe the solution you'd like

github-actions[bot] commented 3 years ago

This issue has not been active since a lot of time. If you think it is still valid, feel free to reply with a simple comment. Without any update, I will close it in 7 days.

Deuchnord commented 3 years ago

Nope, you don't want to close that. :eyes:

github-actions[bot] commented 3 years ago

This issue has not been active since a lot of time. If you think it is still valid, feel free to reply with a simple comment. Without any update, I will close it in 7 days.

nicfb commented 3 years ago

I see that you self-assigned this issue. Are you currently working on this? If not, I would love to give it a shot.

Deuchnord commented 3 years ago

Hello, no, I haven't started it yet, you can take it. Thank you! :smile:

nicfb commented 3 years ago

I am having some issues running using the different output types, specifically on this line. When I use json or text, I have to remove the "b" from the arguments so the file isn't opened in binary mode or else I see this error: TypeError: a bytes-like object is required, not 'str'. I have to add it back to use the pdf output format or else I see the same error. Am I doing something wrong?

Deuchnord commented 3 years ago

No, I think I see what you are facing. It is a bit tricky, since the PDF format is a binary format, but not the plain text and JOSN format. I think the most elegant way to fix this would be to create a small function you would call in the second argument of open() and that would return the correct mode depending on the output format. Something like this:

def get_opening_mode(format: str) -> str:
    if format == "pdf":
        return "wb"

    return "w"

WDYT?

nicfb commented 3 years ago

Ok, just wanted to make sure. I like the solution you suggested!

nicfb commented 3 years ago

I have something working now. I think all that is left is to add some tests. Would I add these in the end to end test script?

Deuchnord commented 3 years ago

I think you can add them just after the existing export ones :)

Deuchnord commented 3 years ago

Btw @nicfb, be careful, there is a license change, Kosmorro is rolling back to GNU AGPL instead of CeCILL (#196). Your PR can stay under CeCILL if you want (AFAIK it is AGPL-compatible), just making sure you are aware about that :)