search

cornell-brg / pymtl

Python-based hardware modeling framework
BSD 3-Clause "New" or "Revised" License
237 stars 82 forks source link

Fix closures #172

Closed orangeturtle739 closed 5 years ago

orangeturtle739 commented 5 years ago

Code like https://github.com/cornell-brg/pymtl/compare/jng55-closures?expand=1#diff-ae2a0fffb8117a39395422133340c31fL29 now works!

cbatten commented 5 years ago

@jsn1993 please take a look and see what you think.

cbatten commented 5 years ago

tell me more about the pytest upgrade. does that mean that anyone that is using pymtl on travis will no need to change their travis.yml file? (i.e., ~100 4750 students?)

orangeturtle739 commented 5 years ago

I don't think everyone who is using pymtl on travis will have to change .travis.yml, but I'm not positive. It seems that the tests for pymtl itself are using some deprecated feature, which was removed in a new version of pytest. But the students in 4750 shouldn't be running the pymtl tests themselves, so they should be OK.

cbatten commented 5 years ago

so what do we need to do next to merge this in?

jsn1993 commented 5 years ago

so what do we need to do next to merge this in?

I'm just asking for spacing fixes. After anyone push a commit to fix all spacings, I will approve.

cbatten commented 5 years ago

right ... i posted a comment about that. for this specific pull request since it is just an update to pymt v2 maybe it is fine as it is?

orangeturtle739 commented 5 years ago

@cbatten @jsn1993 fixed the spacing

jsn1993 commented 5 years ago

LGTM