MRC-CSO-SPHSU / LoneParentsModel.jl

An initial implementation of an ABM for social and child care
0 stars 4 forks source link

Push!(LOAD_PATH,...) #39

Closed AtiyahElsheikh closed 2 years ago

AtiyahElsheikh commented 2 years ago

Having Push!(LOAD_PATH,..) in the code is not good.

When executing the main script couple of times (which is usually my case using REPL and Revise), LOAD_PATH is always incremented with redundant directories. Also the file path is restrictive and the user is enforced to follow a specific directory structure for other Julia packages (e.g. MultiAgents.jl).

The same arguments apply to the code in Person.jl push!(LOAD_PATH, @__DIR__)

mhinsch commented 2 years ago

Ok, I see the point regarding the growth of LOAD_PATH. The directory structure is only hard-coded as long as the other packages are in development and can't be installed properly. Either way, expecting the user to only run the code from the REPL and set LOAD_PATH manually beforehand isn't really a solution either.

AtiyahElsheikh commented 2 years ago

Personally, it has been straightforward for me, because simply push! works with auto-completion under REPL.

No so sure, but there are other couple of ways to avoid setting LOAD_PATH manually every time executing main. Setting the LOAD_PATH globally, or locating the packages under .julia/dev folder?

Edit: I guess the global environment variable is called JULIA_LOAD_PATH.

mhinsch commented 2 years ago

The problem with the repeated changes to LOAD_PATH is pretty straightforward to solve. Either save LOAD_PATH beforehand and reset it afterwards (can be done in the file as well) or - a bit fancier - use something like this (I made some stupid mistakes there, but they are easy to fix). Concerning the directory layout - do you have all of LPM, MultiAgents and Utils in the same directory? If so, that's the same layout I have as well, so in that case I would say let's just keep the hard-coded (relative) paths and change them to something more general when and if necessary.

Edit: Sorry, forgot to reply to your suggestion. Setting a global LOAD_PATH would only solve part of the problem as some of the imports happen locally within LPM.

AtiyahElsheikh commented 2 years ago

I believe that a global solution is much easier to follow.

Here is a typical use-case that I am currently facing (among other use cases)

Here is an actual use-case why relative paths are not encouraged.

I am working on updating RunTests.jl and already running into problems. It is very common to fix a problem after another once a unit test is fail. So I typically need to just import one package, e.g. XAgents: XYZ! from REPL (which won't work because I still need to import the individual modules here and there).

I propose to have a typical package structure:

Module BuildingBlocks.jl that includes

--- KinshipInfo.jl (KinshipBlock type)

--- BasicInfo.jl (BasicInfoBlock type)

mhinsch commented 2 years ago

Is it possible that the previous comment is supposed to go with issue #38? Otherwise I don't understand what you mean.

AtiyahElsheikh commented 2 years ago

After reading issue38, we could have something like

XAgents.AgentsModules.X or similar?

mhinsch commented 2 years ago

To avoid confusion, can we move this discussion over to #38?