byuccl / spydrnet

A flexible framework for analyzing and transforming FPGA netlists. Official repository.
https://byuccl.github.io/spydrnet
BSD 3-Clause "New" or "Revised" License
89 stars 21 forks source link

Zipped support files #200

Closed agg23 closed 1 year ago

agg23 commented 1 year ago

Why are zip files used as checked in assets, seen in support_files? Is the primary goal compression? I see in #153 you mention wanting to save space in the final package, but I would argue that none of these zips should be shipped with the package other than supported architectures.

Storing useful test and runtime information in zips makes it slower for access, and slower for development. There's a lot of semantic information we use via text editors/search, and we can't get that if the files are stored in an opaque structure.

In general, I would advise against storing binary blobs in source control (though it is something you do from time to time). For creation of the published package, the necessarily ziped assets can be assembled at build time, prior to publishing.

jacobdbrown4 commented 1 year ago

Most of the zipped files are example netlists that are "built-in" to spydrnet to allow for easily loading in an example netlist. The thought was that zipping the files would help to reduce the size of the final package. What you are saying about the difficulty of storing these in "an opaque structure" makes sense. Perhaps we should improve this.

agg23 commented 1 year ago

Why do you want to ship example netlists with the package? I am generally unfamiliar with Python's styles, but for most other languages and projects examples are in the repo, but not shipped with the package. It's very strange to me that you would be able to pip install spydrnet and immediately summon an example netlist; I would expect to go download/build that myself, and reference the path in my example script.

jgoeders commented 1 year ago

I agree that test files should not be shipped. This might stem from the fact that the unit tests are being stored within the package. I believe the unit tests and associated test files should be external to the package. See https://stackoverflow.com/questions/5341006/where-should-i-put-tests-when-packaging-python-modules

The test code should be moved out of the spydrnet package directory.

jacobdbrown4 commented 1 year ago

I have moved the tests out of the package on a branch called 'move_tests'. All seems well so I expect to merge this into the main branch sometime.

I guess the next thing to address is all of the example netlists being stored as zipped files in the package. Moving them out would make it more difficult for one to use the 'spydrnet.load_example_netlist_by_name' feature. I don't think this is a huge deal though...just a slight inconvenience for those going through the tutorial and such.

jacobdbrown4 commented 1 year ago

On the 'move_tests' branch, I have moved the zipped example netlists out of the python package and into a separate directory in the repo. I have added support and documentation for this change. I believe it is working okay.

jacobdbrown4 commented 1 year ago

I will merge this into the next release branch and then we will wait to release it later this summer.