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

Include dtype information #35

Merged
merged 8 commits into from
Dec 11, 2021
Merged

Include dtype information #35

merged 8 commits into from
Dec 11, 2021

Conversation

sbrugman
Copy link
Contributor

Closes #25

@parrt
Copy link
Owner

parrt commented Sep 27, 2021

Looking cool...I still need to think about how I'd viz this. Maybe the elem type goes in the box not outside, for example.

@sbrugman
Copy link
Contributor Author

Discussion on that could go in the issue. With respect to implementation: this feature still needs testing, mention in documentation/examples.

@parrt parrt added the enhancement New feature or request label Nov 10, 2021
@parrt parrt added this to the 0.1.4 milestone Nov 10, 2021
@sbrugman
Copy link
Contributor Author

@sbrugman sbrugman marked this pull request as ready for review November 11, 2021 17:46
@parrt
Copy link
Owner

parrt commented Nov 15, 2021

Ok, took me a second, but I managed to find a combination of versions that make the tests pass (python 3.9)! @sbrugman you can pull from master to grab these

@sbrugman
Copy link
Contributor Author

sbrugman commented Dec 4, 2021

Ready for review, legend improved over the last time we spoke

@parrt
Copy link
Owner

parrt commented Dec 4, 2021

sounds good @sbrugman ... I am writing my final exam and hoping to catch up shortly...

@parrt
Copy link
Owner

parrt commented Dec 8, 2021

Trying it out now :)

return head, tail

def _get_dtype_color_indices(self, dtype_name, dtype_precision):
if dtype_name not in self._dtype_name2color:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like base color can change for same type depending on order in which this function is called, right?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll assign a specific color like "greenish" to floats or whatever.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, i'm altering in a copy of your branch so i'll take a whack at fixing. I've made some changes like "no old style shape colors".

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I'm thinking we specify a fixed color per type then bit size is gradation for 4, 8, 16, 32, 64, 128 (auto computed with color map). I'm looking at types for various packages.

@parrt parrt modified the milestones: 0.1.4, 0.2 Dec 11, 2021
@parrt parrt merged commit 4b94320 into parrt:master Dec 11, 2021
@parrt
Copy link
Owner

parrt commented Dec 11, 2021

I have to clean up AST viz but about ready! Thanks for the help!

@parrt parrt modified the milestones: 0.2, 1.0 Dec 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tensor element type info
2 participants