Closed lwasser closed 3 years ago
@lwasser this bug is related to https://github.com/earthlab/abc-classroom/issues/272#issuecomment-693685486 since the folder doesn't exist it can't check the length of the files listed in the folder.
what folder is it looking for @nkorinek ? i ask because the quickstart should make the directory but #272 it was failing when running the quickstart. i wouldnt think it would fail when running abc-quickstart
... super weird.
can we for this template function do a check to ensure extra_files exist and provide a little note that it doesn't exist so it fails gracefully? that seems like a good solution here.
Can do!
Hey @lwasser so in trying to fix this I've realized that this isn't an issue anymore. The line that crashed the code here, nfiles = len(extra_files)
, doesn't exist in abc-classroom
anymore.
I was adding in this check in the quickstart
module, where we discussed we don't need this check anymore, as we decided that we can assume the package was bundled correctly. . I added that check, realized it was in the wrong place, then went over to the template
module to fix it there. However, this line of code no longer exists, it counts the files differently now. The new way that it checks the length of the folder also has a check to make sure the folder exists, and will fail gracefully if it doesn't.
All this to say, I'm fairly sure that in the next release of abc-classroom
, this error has already been addressed and I don't have to open up a PR for it. Let me know if I'm missing something!
ahhhh. ok @nkorinek well how about this. there is a shiny new version of abc-classroom on pypi already. conda forge will come tomorrow. do you want to test both pypi and conda forge for me ?
if all is good we can close this issue and reference the manifest file bundle issue!!
this is great!
i'll create a new issue on docstrings. and will have you work on another (feature) not bug next!
@nkorinek were you able to test this on your computer? it seems to be working for me and if it works for you then i'll close this issue.
Yes, this works locally for me @lwasser !
yay closing. thanks @nkorinek !