search

JuliaDynamics / Agents.jl

Agent-based modeling framework in Julia
https://juliadynamics.github.io/Agents.jl/stable/
MIT License
717 stars 115 forks source link

Enable arrow backend for offline_run! in Windows #909

Closed Tortar closed 9 months ago

Tortar commented 9 months ago

Actually there are current problems with tempfile locking in Windows for Arrow.jl, indeed some tests are not enabled in Arrow.jl for Windows in that repo. I also remembered that offline_run! worked fine for me, there was just a problem in deleting the file during tests but not when using from the external of the package (now I understand that this is due to this locking problem). So I think that we can anyway enable the backend also in Windows so that users can benefit from it.

See https://github.com/apache/arrow-julia/pull/361

codecov-commenter commented 9 months ago

Codecov Report

Merging #909 (b28e679) into main (4750fc0) will increase coverage by 11.43%. Report is 1 commits behind head on main. The diff coverage is n/a.

@@             Coverage Diff             @@
##             main     #909       +/-   ##
===========================================
+ Coverage   80.75%   92.18%   +11.43%     
===========================================
  Files          45       33       -12     
  Lines        2956     2277      -679     
===========================================
- Hits         2387     2099      -288     
+ Misses        569      178      -391
Files Coverage Δ
src/simulations/collect.jl 96.92% <ø> (+0.35%) :arrow_up:

... and 12 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

Datseris commented 9 months ago

@AayushSabharwal could you please be the one that reviews this PR? You have more knowledge on I/O in Agents.jl than me.

Tortar commented 9 months ago

Actually I find IO errors when trying to remove .arrow files even if not in a tempdir, for the rest the backend seems to work fine on Windows, but it's probably better to understand it more deeply this problem before removing the throwing of the error

Tortar commented 9 months ago

Ok, in the end I used a different approach so that now Arrow integration on Windows is actually tested except for the removal of the files, so it should be okay now.

To verify that everything works as expected, I runned the test twice on Windows and the files were actually removed without any problem