Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-39665: Remove faked DataCoordinate from all tests #852

Merged
merged 16 commits into from Jun 16, 2023
Merged

Conversation

timj
Copy link
Member

@timj timj commented Jun 15, 2023

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes

@codecov
Copy link

codecov bot commented Jun 15, 2023

Codecov Report

Patch coverage: 98.49% and project coverage change: -0.03 ⚠️

Comparison is base (a67c57d) 88.01% compared to head (c49c090) 87.98%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #852      +/-   ##
==========================================
- Coverage   88.01%   87.98%   -0.03%     
==========================================
  Files         269      269              
  Lines       35514    35435      -79     
  Branches     7442     7429      -13     
==========================================
- Hits        31258    31178      -80     
- Misses       3113     3114       +1     
  Partials     1143     1143              
Impacted Files Coverage Δ
...lsst/daf/butler/registry/queries/_query_backend.py 92.30% <ø> (ø)
python/lsst/daf/butler/_butler.py 78.96% <75.00%> (-0.09%) ⬇️
tests/test_butler.py 97.77% <83.33%> (+<0.01%) ⬆️
python/lsst/daf/butler/tests/_datasetsHelper.py 88.33% <100.00%> (-0.20%) ⬇️
tests/test_constraints.py 100.00% <100.00%> (ø)
tests/test_datasets.py 100.00% <100.00%> (ø)
tests/test_datastore.py 99.70% <100.00%> (+0.15%) ⬆️
tests/test_formatter.py 100.00% <100.00%> (ø)
tests/test_templates.py 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@timj timj changed the title Remove faked DataCoordinate from all tests DM-39665: Remove faked DataCoordinate from all tests Jun 15, 2023
python/lsst/daf/butler/_butler.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/_butler.py Outdated Show resolved Hide resolved
@@ -91,26 +88,21 @@ def _makeDatasetRef(

if run is None:
run = "dummy"
dataId = DataCoordinate.standardize(dataId, graph=datasetType.dimensions)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that everything conforms in the code calling this I think I can delete this line (I had to add it whilst I was testing things). We don't even annotate this method as taking Mapping for dataId.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to leave it in but I only call it for a dict parameter since we do pass in dicts from the tests.

timj added 7 commits June 15, 2023 09:55
Mostly required adding some band values to DataIds.
Since we now always convert to DataCoordinate these are no
longer needed.
No longer needed now that DatasetRef automatically creates one.
Not mypy clean but this is a start.
@timj timj marked this pull request as ready for review June 15, 2023 17:40
@timj timj requested a review from TallJimbo June 15, 2023 17:40
python/lsst/daf/butler/_butler.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/_butler.py Outdated Show resolved Hide resolved
timj added 3 commits June 16, 2023 12:02
Core butler code should not assume that "instrument" is in
the universe.
Add fake values for the missing dimensions.
@timj timj merged commit a2ef0fb into main Jun 16, 2023
12 of 13 checks passed
@timj timj deleted the tickets/DM-39665 branch June 16, 2023 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants