Open iakovts opened 3 years ago
Hi @iakovts,
Those are some great ideas!
I've always wanted to add some github actions to one of my repos. So far I haven't really taken the time to get to know it very well (I'm personally more comfortable with gitlab CI) so having someone do the first steps would be great! Thanks 😀
All code cleanup is welcome! Just keep in mind that I also want to keep the backend code accessible to scientists/researchers that don't always have a computer science background or are not always as comfortable with python as you or me. Sometimes that means having some code duplication for readability and ease of access. But feel free to make a PR for this, I'm pretty sure there is quite a bit that can be improved 😉
Yep. Sounds good. Alternatively, we could also just use feature branches which we merge into master. But for now I created a dev
branch here.
Hey @flaport,
Hey @iakovts,
Hey @flaport,
So I've started working on the refactoring of boundaries, and as an example I've made some changes to periodic boundaries. These changes are purely "cosmetic", just made to avoid some repetitions of similar classes etc. I would also like to avoid setting self.__class__ = ...
there as I believe it's too implicit, but that would be probably require some major changes (plus I am still not super familiar with the whole package so it could be unavoidable?).
Also, in a completely unrelated note, is self.loc
missing a slice(None)
here ? I was probably going to start working on it too and I noticed it.
thanks @iakovts, this looks good!
I think there should be a way around setting self.__class__
, which I must admit I also don't like very much. I can have a look after merging this.
For numpy arrays slice(None)
at the end of the tuple is equivalent with not adding it at all (e.g. think of x[:, :]
vs x[:]
for a 2D array). However, it's probably better to be more explicit here as well and add it for completeness.
Hey @flaport, sorry for the late response. Oops I didn't realize that for the numpy slices (even though it's something I'm actually using a lot - just didn't put 2 and 2 together till now haha). Ok, I ll keep working on the boundaries and open a PR when ready!
Hello again @flaport,
I 've been reading through the code, and I have suggestions (that I could also possibly implement):
Automate tests through github Actions. (I ve actually already implemented a PoC on my fork - will open a PR). In the future Actions can also be used for publishing docs and the package itself on pypi.
Some (minor?) code refactoring, hopefully without having to change the current API at all. An example of what I have in mind is changing this to a class factory and generally some clarity changes.
[edit] I would also suggest using another branch for active development/merges were changes are frequent (
dev
/staging
?) and usingmaster
as a definitive branch for releases.Looking forward to your thoughts on the above and more :)