search

sensein / b2aiprep

Apache License 2.0
5 stars 6 forks source link

Alistair/resource fixes #59

Closed alistairewj closed 3 months ago

alistairewj commented 3 months ago

Noticed that the snakeCase is not respected in a few files. Not sure how this snuck through as I thought I fixed it all! Added a test to check for it.

alistairewj commented 3 months ago

@ibevers i fixed a few bugs and made sure the tests were passing

having assert NotImplementedError in tests is no fun, hides the real bugs :P

ibevers commented 3 months ago

@alistairewj Ah, yes, I will avoid having NotImplementedError going forward. What do you think of having warnings instead? That way, we are less likely to forget about tests that need to be implemented, but we can still find the real bugs🐛

alistairewj commented 3 months ago

Well that effectively boils down to putting TODOs in the code, and while I don't have unilateral rules, in my opinion would be better to file those TODOs away in an issue tracker, especially if they're not really attached to existing code.

ibevers commented 3 months ago

Ah, I see that makes sense

alistairewj commented 3 months ago

Shall I merge?

alistairewj commented 3 months ago

I don't understand why precancerousLesions.json is deleted here, but I trust that you have a good reason

Yep! I deleted precancerouseLesions.json (note the additional 'e' as a typo). The original file precancerousLesions.json is still there.