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

Make identical products return the same type #77

Merged
merged 6 commits into from
Jul 25, 2019
Merged

Conversation

cdonovick
Copy link
Collaborator

Resolves #76

@cdonovick cdonovick requested a review from rdaly525 July 24, 2019 01:46
@rdaly525
Copy link
Collaborator

rdaly525 commented Jul 24, 2019

@cdonovick, this breaks lassen. I am trying to investigate, but it is definitely when trying to generate magma.

EDIT:
definitely incompatible with the way that magma is doing Product

@leonardt

import magma as m
class A(m.Product):
  a=m.Bit
#the above raises an error in ProductMeta._from_fields

@rdaly525 rdaly525 mentioned this pull request Jul 24, 2019
@cdonovick cdonovick force-pushed the product-cache branch 3 times, most recently from e4fd8ce to 6378a73 Compare July 24, 2019 23:28
@cdonovick cdonovick force-pushed the product-cache branch 2 times, most recently from 0b61527 to d523f53 Compare July 24, 2019 23:50
@leonardt
Copy link
Owner

@rdaly525 this might be fixed by phanrahan/magma@57dc066, can you try it with the latest magma master/release?

@rdaly525
Copy link
Collaborator

Getting a different error now! so closer...

Error from above code:

  File "/Users/rdaly/hwtypes/hwtypes/adt_meta.py", line 241, in __new__
    return mcs._from_fields(fields, name, bases, ns, cache, **kwargs)
TypeError: _from_fields() takes 5 positional arguments but 6 were given

@leonardt
Copy link
Owner

Ah, looks like that interface changed, I'll open a magma PR with the update.

@leonardt
Copy link
Owner

PR is here: phanrahan/magma#427, running the lassen test suite now, seems to be making good progress

@rdaly525
Copy link
Collaborator

@leonardt once that PR is in, the GarnetFlow should be fixed for this PR

@rdaly525
Copy link
Collaborator

@cdonovick Can you add the exact test from #74? Other than that looks good.

@cdonovick
Copy link
Collaborator Author

Now also resolves #74

@cdonovick cdonovick force-pushed the product-cache branch 2 times, most recently from 6fedb00 to 089b556 Compare July 25, 2019 20:17
@cdonovick cdonovick merged commit 1868ebb into master Jul 25, 2019
@cdonovick cdonovick deleted the product-cache branch July 25, 2019 21:46
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 this pull request may close these issues.

[feature-request] Product types defined the same way should be the same object.
3 participants