OPSIM-1079: Update pyproject.toml and workflows #364
Conversation
|
I expect that "Build and Upload Docs" will fail until we move from our current doc setup to the Rubin User Guide setup, but since I was adding other workflows here I just added it anyway. Could pull it out until later. Currently the "Run Tests and Build Documentation" does build and publish the docs, and so the publish docs part of that would go away after the doc update, in favor of the newer Build and Upload docs workflow. |
|
The "build_pypi" workflow won't be triggered until the PR is merged to main, and won't publish to pypi unless there is a tag. |
|
For Eric - I think this would put rubin_sim onto pypi (as rubin-sim or rubin_sim? not sure yet) with dependencies, except that the pyoorb part wouldn't be usable without an additional (only condo-installable) piece of data. I don't think you're using that module though in schedview. |
There was a problem hiding this comment.
There are numerous instances where the old version had an assignment, but the new version just has a bare calculation that just throws the value away, e.g. somithing like
t = time.time()
became just
time.time()
I speculate that this was done by ruff or black after it noticed that the assigned value wasn't used, but that it lacked the confidence to remove the calculation itself because it didn't know that there weren't desired side-effects. I marked a few examples, but there are many more, and someone should probably go through the diff and just get rid of them.
Also, I notice in same places that the statement of default values in docstrings is remoeb, I assume to make the line length limits. Default are, however, useful in docstrings, so perhaps they should have been kept (just put on a different line).
| @@ -168,9 +169,9 @@ def astrometryBatch( | |||
| Parameters | |||
| ---------- | |||
| colmap : `dict` or None, optional | |||
| A dictionary with a mapping of column names. Default will use OpsimV4 column names. | |||
| A dictionary with a mapping of column names. | |||
There was a problem hiding this comment.
In this an other instances, removing statement of what the default is may not be the best idea.
Perhaps just put it on a new line.
| d_t1grid = info_dict["dT1s"][abs(d_t1 - info_dict["dT1s"]).argmin()] | ||
| d_t2grid = info_dict["dT2s"][abs(d_t2 - info_dict["dT2s"]).argmin()] | ||
| info_dict["dT1s"][abs(d_t1 - info_dict["dT1s"]).argmin()] | ||
| info_dict["dT2s"][abs(d_t2 - info_dict["dT2s"]).argmin()] |
There was a problem hiding this comment.
Do these lines actually do anything?
| @@ -187,7 +187,7 @@ class ValueAtHMetric(BaseMoMetric): | |||
|
|
|||
| def __init__(self, h_mark=22, **kwargs): | |||
| metric_name = "Value At H=%.1f" % (h_mark) | |||
| units = "<= %.1f" % (h_mark) | |||
| "<= %.1f" % (h_mark) | |||
There was a problem hiding this comment.
Does this line do anything?
| @@ -176,7 +176,7 @@ def process(self, sel): | |||
| """ | |||
|
|
|||
| self.band = np.unique(sel[self.filter_col])[0] | |||
| time_ref = time.time() | |||
| time.time() | |||
There was a problem hiding this comment.
Again, why is this here?
|
Jon had suggested that default values not be stated in the doc string, because you can see the value in the docs in the call. |
|
I do seem to have some unit test failures that got triggered by the ruff though, so will fix those. |
| @@ -164,7 +164,7 @@ def run(self, data_slice, slice_point=None): | |||
| # Sort on time, to be sure we've got filter (or other col) changes in the right order. | |||
| idxs = np.argsort(data_slice[self.time_col]) | |||
| changes = data_slice[self.change_col][idxs][:-1] != data_slice[self.change_col][idxs][1:] | |||
| condition = np.where(changes == True)[0] | |||
| condition = np.where(changes is True)[0] | |||
There was a problem hiding this comment.
ruff is a little over-eager about these replacements. The changes actually make the metrics fail.
…user guide format)
… and non-maintained (transient_ascii_sed) files.
rhiannonlynne
force-pushed
the
tickets/OPSIM-1079
branch
from
4aacfa6
to
634f852
Compare
Update pyproject.toml to fix setuptools_scm issue with version.py file
update workflows - add ruff workflow, move rubin-sim tests to mamba forge
set up for pypi installation of rubin-sim