DM-33049: Allow duplicate values in dataId #622
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
timj
force-pushed
the
tickets/DM-33049
branch
from
b6e1329
to
152a9bf
Compare
Codecov Report
@@ Coverage Diff @@
## main #622 +/- ##
==========================================
+ Coverage 84.14% 84.18% +0.04%
==========================================
Files 237 237
Lines 30305 30327 +22
Branches 5008 5015 +7
==========================================
+ Hits 25499 25532 +33
+ Misses 3661 3652 -9
+ Partials 1145 1143 -2
Continue to review full report at Codecov.
|
timj
force-pushed
the
tickets/DM-33049
branch
from
152a9bf
to
59cd3a5
Compare
Previously non-dimensions were removed from kwargs and valid dimensions were retained. None of this agreed with the definition of "unused" in the docstring since those now raise an exception (and were already removed from kwargs). It's much more consistent to transfer kwargs over to the new dataId and empty kwargs.
We allows duplicate information in the dataId but if there is duplicate information it should be checked for consistency.
This is better than a RuntimeError and consistent with other cases where the dataId does not resolve.
When trying to match up record values to a specific dimension, there are sometimes cases where the dimension is given as well as record values. In that case we need to also assign the records to a dimension for later resolution. Without doing this the records look like unrecognized keywords.
day_obs and exposure can be given and if they are consistent this should not raise an exception.
timj
force-pushed
the
tickets/DM-33049
branch
from
59cd3a5
to
f1f2cde
Compare
If day_obs+seq_num have been given as well as explicit exposure then a ValueError should also be raised during the check for consistency if the exposure is not known at all.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
detectorandraftandday_obsandexposureshould be allowed so long as they are consistent.Checklist
doc/changes