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

Fixes ecocal command #2265

Merged
merged 3 commits into from
Aug 8, 2022
Merged

Fixes ecocal command #2265

merged 3 commits into from
Aug 8, 2022

Conversation

DidierRLopes
Copy link
Collaborator

Fixes #2248

Screenshot 2022-08-07 at 23 08 16

@DidierRLopes DidierRLopes added the bug Fix bug label Aug 7, 2022
@jmaslek
Copy link
Collaborator

jmaslek commented Aug 8, 2022

Can we rename the function from ecocal->econcal?

@montezdesousa
Copy link
Contributor

montezdesousa commented Aug 8, 2022

Can we rename the function from ecocal->econcal?

I don't love ecocal, but couldn't come up with better name at the time. I'm ok with econcal, but what do you think of something smaller/cleaner like events or agenda?

@montezdesousa
Copy link
Contributor

Changes looks good to me, it actually fixes a still to be found issue with filtering by categories with spaces. Apparently investpy uses _ in those cases.

not good: investpy.news.economic_calendar(time_filter = "time_only", categories=["economic activity"])
good: investpy.news.economic_calendar(time_filter = "time_only", categories=["economic_activity"])

@DidierRLopes
Copy link
Collaborator Author

I added the "_" to not have the issue with 2 words but after I replace the underscore by a space. Unfortunately, we can't use choices= when nargs="+" which is annoying :/

@montezdesousa if we merge this can you then change the function name from ecocal toevents ?

@montezdesousa
Copy link
Contributor

montezdesousa commented Aug 8, 2022

I added the "_" to not have the issue with 2 words but after I replace the underscore by a space. Unfortunately, we can't use choices= when nargs="+" which is annoying :/

@montezdesousa if we merge this can you then change the function name from ecocal toevents ?

Seen, didn't test it. So we can just remove that replace and should work
yes

@DidierRLopes DidierRLopes merged commit b7a84af into main Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fix bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] /economy/ecocal --category flag is broken. Argparse is not joining words.
3 participants