search

yvs314 / epi-net-m

Data Processing and Simulation Tools for Networked SIR+
MIT License
1 stars 0 forks source link

Modularization of Oboe.jl #19

Closed karaign closed 3 years ago

karaign commented 3 years ago
  1. I'm not exactly sure how to handle the versions. As per SemVer, breaking API changes require a major version bump, but should this only apply to Oboe.jl or every file in the project? What about the submodules?
  2. Still haven't tested the performance effect of this, a Julia update broke my Jupyter setup and I am trying to get it to work again. This is also why I haven't yet updated mkpsgmx-benchmark to work with the API change.
yvs314 commented 3 years ago
  1. I'm not exactly sure how to handle the versions. As per SemVer, breaking API changes require a major version bump, but should this only apply to Oboe.jl or every file in the project? What about the submodules?

Bump the Oboe.jl major version (and do retain the old changelog), but treat the submodules in the project as brand-new. In the in-file changelogs for submodules, put something like “2021-08-XX v.0.1 first modular edition” and anything meaningful after that. Third case, the already existing oboe-cli.jl and oboe-main.jl: do not treat this as major version, bump the minor with something like ”2021-08-XX v._.(+1) reskin into FromFile-style module, change XXXXX”.

Also, great work on docstrings!

  1. Still haven't tested the performance effect of this, a Julia update broke my Jupyter setup and I am trying to get it to work again. This is also why I haven't yet updated mkpsgmx-benchmark to work with the API change.

Yeah, I appreciate how Julia updates sometimes are. I went through an update of DataFrames.jl that deprecated certain grouping and join syntax (v.0.9.[2,3]). And then there was some Anaconda stuff that wouldn't let it update itself without a clean reinstall.

Minor issues/requests

1. Oboe.jl: version bump to 1.1 (or further) not reflected in callsign

2. airports.jl: some default args that may trigger IO still remain

Existing chaining by default arguments complicates the control flow, which should be made explicit in oboe-main.jl instead of currently implicit chaining in airports.jl. Sorry about the mess. It is OK to merge some of the smaller functions that are only ever used together instead of explicitly composing them. Alternatively, it is OK to write this composition as part of airports.jl if you feel it would muddy oboe-main.jl.

karaign commented 3 years ago

@yvs314 I've updated the versions and addressed the airports.jl issue, please have a look.