MapReader icon indicating copy to clipboard operation
MapReader copied to clipboard

Unit tests fail

Open geekysquirrel opened this issue 11 months ago • 5 comments

Describe the bug Tests fail due to dependencies not being installed during normal installation (timm, transformers). If the test environment is different from the runtime environment, I would expect instructions on how to install and run tests but I can't find separate instructions on running tests. That said even after installing all missing dependencies, tests still fail.

To Reproduce pytest .

Expected behavior tests should pass

Screenshots Screenshot from 2024-03-17 18-46-53

Desktop (please complete the following information):

  • OS: Ubuntu
  • Version 22.04

geekysquirrel avatar Mar 17 '24 18:03 geekysquirrel

Hi, thanks for flagging this.

Currently timm and transformers are in our dev dependencies and installed using pip install "mapreader[dev]" (PyPI) or `pip install ".[dev]" (if installing from source). These are used for loading models when testing the classifier but aren't required if you load a model from torch so are in the dev dependencies vs requirements. In the docs we then say to install them separately.

Happy to take advice on this of whether this seems like a sensible set up or would be better for them to be requirements. Also happy to add this to the docs.

rwood-97 avatar Mar 18 '24 08:03 rwood-97

Also re. tests failing, can you confirm mapreader version so I can check this?

rwood-97 avatar Mar 18 '24 09:03 rwood-97

Hi, just thinking this through and suspect if you have installed mapreader 1.1.0 but are testing with most recent version of mapreader repo then this is likely the problem. I will try build in some backwards compatibility and deprecation warnings to fix this but it should also be resolved by checking out the commit corresponding to your mapreader version. For 1.1.0 8997b4d should be the right commit

rwood-97 avatar Mar 18 '24 09:03 rwood-97

Thanks for explaining the dependency situation. I didn't dive that deep into the docs to find the note about installing the libraries separately. pip install "mapreader[dev]" worked just fine though and it makes total sense to add them only as dev deps. Perhaps that info needs to be a bit more prominent but I leave that up to you.

As for the tests I just pulled the latest code and upgraded the python package using pip install mapreader -U which updated the version from 1.1.4 to 1.1.5. I still get failing tests, albeit one less than before :) image

geekysquirrel avatar Apr 07 '24 18:04 geekysquirrel

Hi,

I can't reproduce this. My steps were:

  • set up a new conda env with python 3.9
  • pip install "mapreader[geo, dev]" to get mapreader v1.1.5
  • python -m pytest . gives me 188 passed

Things I would suggest (sorry if these are obvious and you have tried them already):

  • git checkout main and then git pull to check you have most recent version of test files
  • git clean -di to clean git repo just incase
  • find . -name '__pycache__' -exec rm -rf {} + to remove pycache
  • set up new clean conda env and start installation again

Then also, googling the error suggests it could be to do with a corrupted checkpoint file, e.g. this and this. I guess this is related to the resnet18 torch model but unsure how to fix.

rwood-97 avatar Apr 08 '24 10:04 rwood-97

hey, could you try these again with the most recent mapreader?

rwood-97 avatar May 09 '24 12:05 rwood-97

I think this is resolved with the fix of using the most recent mapreader version. But would be great for @geekysquirrel to confirm.

kmcdono2 avatar May 29 '24 06:05 kmcdono2

👋 Hi @geekysquirrel, I just wanted to follow-up on this. Please let us know when you're able to re-review this issue !

emdupre avatar Jun 04 '24 17:06 emdupre

Hi everyone, I just wanted to follow-up that I've confirmed with @geekysquirrel via email that they're happy with the current status of the codebase and are OK to move forward.

I'll thus consider this issue closed for the purposes of the JOSS review !

emdupre avatar Jun 12 '24 19:06 emdupre