Closed dfm closed 3 years ago
@benjaminpope: One question regarding the recommended citations: What is the best reference for PyExoTk
? All I can find is this GitHub repo and it doesn't even have a LICENSE so I'm hesitant to cite it.
Good question. I recall using it years ago but maybe it was never published. Did it get eaten up by PyTransit or RoadRunner maybe? Can probably ignore this recommendation, it was just the first thing that came to mind from my own experience. ldtk at least is well-documented.
The remaining point that hasn't been addressed or given an issue is the suggestion of adding a figure to the paper. I'm somewhat hesitant since the paper is already much longer than a typical JOSS paper and I'm having trouble selecting a figure that would add to the paper meaningfully. But, I'm going to revisit that after addressing the other issues.
The remaining point that hasn't been addressed or given an issue is the suggestion of adding a figure to the paper. I'm somewhat hesitant since the paper is already much longer than a typical JOSS paper and I'm having trouble selecting a figure that would add to the paper meaningfully. But, I'm going to revisit that after addressing the other issues.
How about a compilation of the case studies?
@ericagol: That's a good option, but I still feel like it would just be taking up space without adding much... but it probably wouldn't hurt!
In this PR, I'll make the small suggested changes from @benjaminpope's JOSS review or link to issues/PRs tracking the larger changes. I'll update this comment as I go.
Unit tests:
test_small_star
andtest_sky_coords
. This was because I hadn't yet installed batman - if these are going to be in unit tests perhaps they should also be in dependencies. batman was included in the "test" extras but I've switched these tests to skip when batman isn't installed, making it an optional dependency to smooth issues like this. batman will still be installed an tested on CI.Case Studies:
WARNING (theano.tensor.opt): Cannot construct a scalar test value from a test value with no size: InplaceDimShuffle{x,0}.0
WARNING (theano.tensor.opt): Cannot construct a scalar test value from a test value with no size: Elemwise{cos,no_inplace}.0
Tracked in https://github.com/exoplanet-dev/case-studies/issues/18General paper comments:
exoplanet
. The multi-instrument light curve and RV fits from "Fitting light curves from multiple instruments" and "RVs with multiple instruments" respectively could go well as a multi-panel figure. (This is by no means a requirement for the paper to be accepted, just an opportunity). I have added a figure.ldtk
(I list these because they are the ones I have used myself; wide citation is to be encouraged, and perhaps the authors can think of others). Given the detached EB model, perhaps a citation to Irwin'seb
or thePHOEBE
orJKTEBOP
projects? Done. I'm not including a citation to PyExoTk because it doesn't seem to have one or be licensed.