simple things first

[BMaxV]

heyo, first pull request, yay.

It's just touching the setup. Why tried installing the module it downloaded old versions of these modules because you had specified it had to be that exact version. Namely numpy. I don't think that's necessary. Also the requirements that are checked in the status kind of should be the requirements the package is actually installing.

It's fairly self explanatory really.

I've done more, changing the cli.main pretty significantly. Mostly encapsulation up to now.

I will add json dumps for some parts. Protobuf seems very powerful, but as far as I can see, a dumped dict will serve the same purpose and those will be way handier inside python. Anyway I'll show that off when it's done.

[coveralls]

Coverage remained the same at ?% when pulling e6d3fb7b551dbdda8d6af1575a1fcc5db44201dc on BMaxV:master into 1db5b2b0aeae04081ca68e80b7bc08d76bdf3192 on Mindwerks:master.

[coveralls]

Coverage remained the same at 84.833% when pulling 739a3e3d2106709a704fb6986146f55fb406166d on BMaxV:master into 1db5b2b0aeae04081ca68e80b7bc08d76bdf3192 on Mindwerks:master.

[codecov-io]

Codecov Report

Merging #257 into master will decrease coverage by 4.92%. The diff coverage is 58.44%.

@@            Coverage Diff             @@
##           master     #257      +/-   ##
==========================================
- Coverage   79.49%   74.57%   -4.93%     
==========================================
  Files          28       28              
  Lines        3468     3524      +56     
  Branches      651      652       +1     
==========================================
- Hits         2757     2628     -129     
- Misses        526      708     +182     
- Partials      185      188       +3

Impacted Files	Coverage Δ
worldengine/plates.py	58.69% <100%> (-39.14%)	:arrow_down:
worldengine/hdf5_serialization.py	86.39% <100%> (ø)	:arrow_up:
worldengine/world.py	74.43% <100%> (ø)
worldengine/generation.py	83.12% <100%> (-8.13%)	:arrow_down:
worldengine/save.py	13.63% <13.63%> (ø)
worldengine/main.py	20.16% <20.16%> (ø)
worldengine/simulations/erosion.py	59.28% <25%> (ø)	:arrow_up:
worldengine/cli.py	78.13% <78.13%> (ø)
worldengine/imex/__init__.py	0% <0%> (-10%)	:arrow_down:
... and 10 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1db5b2b...e6d3fb7. Read the comment docs.

[psi29a]

You've broken all the tests.

We pin versions for a reason, to make sure that we're on the same page and that CI builds keep working. The last thing we need is for every CI build to be building against a different version of a library. That is frustrating when trying to debug issues as you don't know if it is your code change or the new library.

Newest is not best. In the latest libraries, are there bug fixes that fixes things in our code? Are there new features that we use? Are you getting the latest and greatest because you want the latest and greatest? :)

[BMaxV]

Currently the builds (and probably the tests) fail because I rewrote stuff and didn't check if things worked before pushing.

I am getting the latest because the latest will hopefully not conflict with other packages. I don't plan to use worldengine in a vacuum / venv.

[BMaxV]

Ok so far so good, idk what pyflakes does but apparently it's failing somehow. If you could look into that and tell me what broke, I can try to fix it.

Despite the builds failing, the program actually works and produces output for me and the different operations are a bit better encapsulated, I think.

[psi29a]

Can you see the output of pyflakes? It tells you what is wrong, you can run it locally too.

https://travis-ci.org/Mindwerks/worldengine/jobs/337775382

[BMaxV]

ERROR: InvocationError: '/home/travis/build/Mindwerks/worldengine/.tox/pyflakes/bin/pyflakes worldengine'
___________________________________ summary ____________________________________
ERROR:   pyflakes: commands failed

This is what I can see and would identify as an actual error. I assume pyflakes tries to call worldengine and nothing happens? Not sure why that is the case. It's probably because of what I did to the setup but I don't know which part of what I did broke this.

The unused definitions are clean up work that I can and will do later.

[psi29a]

That's because you purged: worldengine/__main__.py

This is there so that you can call python worldengine like a first class application. That is your starting point.

[BMaxV]

Ok I'm confused. At least the serialization test passes on my machine and doesn't on travis. All failures seem to be the same kind of bug too.

[BMaxV]

I think some stuff I found merits a comment: the same kind of noise is used multiple times, but not as a function, you're just generating the noise multiple times the same way. As far as I could the permeability is just noise too.

I'm not done with the biomes yet, those are really confusing, the thresholds are basically filter matrices, but at the biome step you just ignore them and create a new matrix and assign strings to every cell? Especially since the project is so concerned with efficiency and testing, that seems really wasteful memory wise.

[BMaxV]

Also from the looks of it, you're randomizing the distance from the planet to it's sun and then you normalize the temperatures later to create more interesting biomes overall.

[ftomassetti]

I have the feeling this PR is getting to large: 31 commits are a lot. Would be easier to start with a few changes and get those approved first?

[BMaxV]

It is getting pretty large. I'm not sure I could split the changes in other ways and gradually integrate them, the code isn't modular enough that I could just swap out parts. Wouldn't be easy.

[Asday]

I don't plan to use worldengine in a vacuum / venv.

That's exactly how all Python is used in production though. What reason do you have to try and break that norm with this that will improve its use for everyone else?

[BMaxV]

@Asday thanks for the reminder

I'm no longer interested in putting work into getting something merged here.

[Asday]

No worries. I don't even understand what your intention was after reading through the code - I hope you haven't been soured on contributing to open source.

[BMaxV]

I would have to look at it in detail again to understand my changes tbh, and I want to do different things at the moment.

Open source is still a great idea, I just have to live with it that sometimes trying to contribute doesn't work out, without anyone to blame. Here, I clearly did too much too fast. And breaking tests isn't a good idea.

I disagree with some things about the architecture, and I stand by not using venvs.

What reason do you have to try and break that norm with this that will improve its use for everyone else?

Ideally all parts should be subprograms (I think this was something I changed) that do one thing in isolation and very well?

So if I want to use a part of this somewhere else, the idea of keeping separate versions of e.g. numpy in memory just because multiple people pinned different versions is silly to me. Maybe that is silly because it's so little memory nowadays.

[Asday]

I still don't see it in the code, but from your words I'm starting to understand. I think you might benefit from a little elucidation upon the divide between applications vs libraries. An application is intended to be the end point - it's the final strings and clue holding other more generally useful code together in a way that's only useful in its own specific context. Worldengine is an application, and as such it doesn't want to have to deal with its dependencies updating and changing their behaviour. Generally an application maintainer only wants to update dependencies when a security release is made, or a new feature is required. Django is a library - it's not intended to be run as an application on its own - and as such, it's much freer with its requirement constraints, to make it harder for an application developer to get into a situation where one of their dependencies depends upon X version 1 or below, and another depends upon X version 2 or above.

There are generally two ways to deal with dependencies - shared libraries or static libraries. In Python, we've accepted that people write code that breaks backwards compatibility all the time, and (possibly due to Python's openness) people will write code that depends upon specific behaviour in its dependencies, even if maybe it shouldn't. As such, we go with static libraries, in the form of virtual environments for applications. I can't tell you how many issues I've personally had to fix that were caused by dependencies not being pinned to a specific version, then someone running pip install -r requirements.txt (or npm i or whathaveyou), breaking everything, because new versions with different behaviour have been released.

It's worth nothing that this choice isn't specific to Python. From what I understand, macOS programs are a self-contained bundle of application code and libraries, and docker is extremely popular mostly because it practically enforces isolated environments.

If you want to portion parts of worldengine out to be used as dependencies in other things, rather than as an executable in and of itself, then that's a fantastic cause, but the way to do that isn't to unpin dependencies. Instead what you should do is create a new repo (perhaps like this to retain git history) using worldengine code, and strip it back to just one piece of functionality with much looser dependency requirements, (usually a major version restraint only), cover it with extra tests as required, write docs, and then open a PR to worldengine with the portioned out library removed and replaced with calls to it.

I'd like to point out that the above is something very useful even in professional settings. Team A writes some functionality that does something, Team B does very similar, manager C notices that there are some inconsistencies between the teams' workflows and says "this needs to be made its own thing so everyone's on the same page, easier to train new people, work isn't duplicated, etc." Really good managers also get the package open sourced, but hey.

[BMaxV]

Thanks for the in depth reply, that must have taken some time to write. I really appreciate it. I have actual problems with some packages atm and written bug reports that no one seems to care about, so this does give back some hope that it's worthwhile in the end.

but the way to do that isn't to unpin dependencies. Instead what you should do is create a new repo (perhaps like this to retain git history) using worldengine code, and strip it back to just one piece of functionality with much looser dependency requirements, (usually a major version restraint only), cover it with extra tests as required, write docs, and then open a PR to worldengine with the portioned out library removed and replaced with calls to it.

This is the proper way to do it.

Although how easy or possible it is sometimes depends on the code as it is. I don't remember how open worldengine's code would be to do it.

I will probably come back to worldengine at some point and reuse some parts, we'll see if I can contribute something then.

[Asday]

Yeah definitely, most things are written with only the current use in mind, (see YAGNI), and can as such become woven into the fabric of the project, not being easily extricable. Not that your manager will care, of course, when it comes to do it in your company. Sometimes stuff is hard, and there's not a great deal that can be done about that.

If you find it too hard to drag some useful code from worldengine into its own spot, have a poke around other applications (or even libraries) that you currently use and see if anything jumps out at you. I know there are a couple of things I'd like to see made their own thing within Django that I'll probably do one day, for instance.