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

Cleanup visualisation code prior to 0.1 release #265

Closed
2 tasks done
cjayb opened this issue Jan 30, 2021 · 2 comments · Fixed by #264
Closed
2 tasks done

Cleanup visualisation code prior to 0.1 release #265

cjayb opened this issue Jan 30, 2021 · 2 comments · Fixed by #264
Milestone

Comments

@cjayb
Copy link
Collaborator

cjayb commented Jan 30, 2021

I strongly suggest we make decisions on these points before releasing. I fear they'll haunt us otherwise!

On the first point, I propose changing to postproc=False (and modifying all tests to explicitly set it to True). I think this should be the case regardless of what the default behaviour of HNN GUI needs to be.

On the second, I like @jasmainak 's proposal to keep everything in fAm, and use scalings-arguments to plot in given units. @rythorpe also pointed out that legends can be used to indicate what amount of scaling has been applied.

These seem like two quite straightforward PRs to me, should we include them in 0.1?

@cjayb cjayb added this to the 0.1 milestone Jan 30, 2021
@jasmainak
Copy link
Collaborator

I am okay with this but we might need a section in the documentation to explain how to get the "old behavior" and explaining to folks what has changed and why. It's partly there in the whats_new but maybe worth pulling it out into a separate document might be helpful. Mostly to get buy-in from folks who have been using HNN-GUI in the past.

For postproc, another option is to set postproc=True but have a deprecation warning that next cycle it will become postproc=False. But probably not worth the effort

@ntolley
Copy link
Contributor

ntolley commented Feb 1, 2021

This may not be worth taking the time to implement but the quantities package is definitely aimed at this exact issue of units.

I've seen it implemented in a few neuroscience specific toolkits (neo and elephant specifically), but at the moment it is tough to say how many headaches it would save.

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

Successfully merging a pull request may close this issue.

3 participants