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

remove freq option in matrix #154

Open
majidaldo opened this issue Mar 8, 2022 · 1 comment
Open

remove freq option in matrix #154

majidaldo opened this issue Mar 8, 2022 · 1 comment

Comments

@majidaldo
Copy link

majidaldo commented Mar 8, 2022

I'm seeing all the issues related to this option. I'm having issues with KeyError('Could not divide time index into desired frequency.'). The solution is to just remove this option. The index is the user's responsibility and the library should take it as is.

There should be a separate data structing step which would be followed by the viz.

@ResidentMario
Copy link
Owner

I think this feature is useful enough to warrant inclusion, but I concede that it's not the best thought-out library feature. Such are the pitfalls of accepting pull requests from strangers.

Rather than removing it outright, I plan to audit it and see what I can do the next time I work on this codebase. I suspect that I can get this feature to a good state with more careful error handling and strftime formatting for the labels.

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

No branches or pull requests

2 participants