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 47944: Fix calibration summary bug in v28 #1126

Merged
merged 1 commit into from
Dec 6, 2024
Merged

Conversation

mwittgen
Copy link
Contributor

@mwittgen mwittgen commented Dec 5, 2024

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes
  • (if changing dimensions.yaml) make a copy of dimensions.yaml in configs/old_dimensions

Without this fix, calibration dataset types that were registered in
data repositories where the only existing dataset types with those
dimensions were not calibrations would incorrectly be registered as
non-calibrations.
@mwittgen mwittgen requested a review from TallJimbo December 5, 2024 03:16
@mwittgen mwittgen changed the base branch from main to v28.0.x December 5, 2024 03:17
Copy link

codecov bot commented Dec 5, 2024

❌ 11 Tests Failed:

Tests completed Failed Passed Skipped
1845 11 1834 14
View the top 3 failed tests by shortest run time
tests/test_formatter.py::ZipFormatterTestCase::test_metrics
Stack Traces | 0.002s run time
self = <test_formatter.ZipFormatterTestCase testMethod=test_metrics>

    def test_metrics(self):
        from lsst.daf.butler.tests import MetricsExample
        from lsst.daf.butler.tests.testFormatters import MetricsExampleFormatter
    
        formatter = self._make_formatter(
            MetricsExample, MetricsExampleFormatter, "formatter_tests/metrics.yaml"
        )
>       metrics = formatter.read()

tests/test_formatter.py:325: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = lsst.daf.butler.tests.testFormatters.MetricsExampleFormatter(FileDescriptor(Location(None, '.../tests/data/formatter_tests.zip'), StorageClass('Something', pytype='lsst.daf.butler.tests.MetricsExample')))
component = None, expected_size = -1, cache_manager = None

    def read(
        self,
        component: str | None = None,
        expected_size: int = -1,
        cache_manager: AbstractDatastoreCacheManager | None = None,
    ) -> Any:
        """Read a Dataset.
    
        Parameters
        ----------
        component : `str`, optional
            Component to read from the file. Only used if the `StorageClass`
            for reading differed from the `StorageClass` used to write the
            file.
        expected_size : `int`, optional
            If known, the expected size of the resource to read. This can be
            used for verification or to decide whether to do a direct read or a
            file download. ``-1`` indicates the file size is not known.
        cache_manager : `AbstractDatastoreCacheManager`
            A cache manager to use to allow a formatter to cache a remote file
            locally or read a cached file that is already local.
    
        Returns
        -------
        in_memory_dataset : `object`
            The requested Dataset.
    
        Notes
        -----
        This method should not be subclassed. Instead formatter subclasses
        should re-implement the specific `read_from_*` methods as appropriate.
        Each of these methods has a corresponding class property that must
        be `True` for the method to be called.
    
        The priority for reading is:
    
        * `read_from_uri`
        * `read_from_stream`
        * `read_from_local_file`
        * `read_from_uri` (but with a local file)
    
        Any of these methods can return `NotImplemented` if there is a desire
        to skip to the next one in the list. If a dataset is being requested
        with no component, no parameters, and it should also be added to the
        local cache, the first two calls will be skipped (unless
        `read_from_stream` is the only implemented read method) such that a
        local file will be used.
    
        A Formatter can also read a file from within a Zip file if the
        URI associated with the `FileDescriptor` corresponds to a file with
        a `.zip` extension and a URI fragment of the form
        ``zip-path={path_in_zip}``. When reading a file from within a Zip
        file the priority for reading is:
    
        * `read_from_stream`
        * `read_from_local_file`
        * `read_from_uri`
    
        There are multiple cases that must be handled for reading:
    
        For a single file:
    
        * No component requested, read the whole file.
        * Component requested, optionally read the component efficiently,
          else read the whole file and extract the component.
        * Derived component requested, read whole file or read relevant
          component and derive.
    
        Disassembled Composite:
    
        * The file to read here is the component itself. Formatter only knows
          about this one component file. Should be no component specified
          in the ``read`` call but the `FileDescriptor` will know which
          component this is.
        * A derived component. The file to read is a component but not the
          specified component. The caching needs the component from which
          it's derived.
    
        Raises
        ------
        FormatterNotImplementedError
            Raised if no implementations were found that could read this
            resource.
        """
        # If the file to read is a ZIP file with a fragment requesting
        # a file within the ZIP file, it is no longer possible to use the
        # direct read from URI option and the contents of the Zip file must
        # be extracted.
        uri = self.file_descriptor.location.uri
>       if uri.fragment and uri.unquoted_fragment.startswith("zip-path="):
E       AttributeError: 'FileResourcePath' object has no attribute 'unquoted_fragment'

.../daf/butler/_formatter.py:442: AttributeError
tests/test_formatter.py::ZipFormatterTestCase::test_packages
Stack Traces | 0.003s run time
self = <test_formatter.ZipFormatterTestCase testMethod=test_packages>

    def test_packages(self):
        from lsst.daf.butler.formatters.packages import PackagesFormatter
        from lsst.utils.packages import Packages
    
        formatter = self._make_formatter(Packages, PackagesFormatter, "formatter_tests/packages.yaml")
>       packages = formatter.read()

tests/test_formatter.py:314: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = lsst.daf.butler.formatters.packages.PackagesFormatter(FileDescriptor(Location(None, '.../tests/data/formatter_tests.zip'), StorageClass('Something', pytype='lsst.utils.packages.Packages')))
component = None, expected_size = -1, cache_manager = None

    def read(
        self,
        component: str | None = None,
        expected_size: int = -1,
        cache_manager: AbstractDatastoreCacheManager | None = None,
    ) -> Any:
        """Read a Dataset.
    
        Parameters
        ----------
        component : `str`, optional
            Component to read from the file. Only used if the `StorageClass`
            for reading differed from the `StorageClass` used to write the
            file.
        expected_size : `int`, optional
            If known, the expected size of the resource to read. This can be
            used for verification or to decide whether to do a direct read or a
            file download. ``-1`` indicates the file size is not known.
        cache_manager : `AbstractDatastoreCacheManager`
            A cache manager to use to allow a formatter to cache a remote file
            locally or read a cached file that is already local.
    
        Returns
        -------
        in_memory_dataset : `object`
            The requested Dataset.
    
        Notes
        -----
        This method should not be subclassed. Instead formatter subclasses
        should re-implement the specific `read_from_*` methods as appropriate.
        Each of these methods has a corresponding class property that must
        be `True` for the method to be called.
    
        The priority for reading is:
    
        * `read_from_uri`
        * `read_from_stream`
        * `read_from_local_file`
        * `read_from_uri` (but with a local file)
    
        Any of these methods can return `NotImplemented` if there is a desire
        to skip to the next one in the list. If a dataset is being requested
        with no component, no parameters, and it should also be added to the
        local cache, the first two calls will be skipped (unless
        `read_from_stream` is the only implemented read method) such that a
        local file will be used.
    
        A Formatter can also read a file from within a Zip file if the
        URI associated with the `FileDescriptor` corresponds to a file with
        a `.zip` extension and a URI fragment of the form
        ``zip-path={path_in_zip}``. When reading a file from within a Zip
        file the priority for reading is:
    
        * `read_from_stream`
        * `read_from_local_file`
        * `read_from_uri`
    
        There are multiple cases that must be handled for reading:
    
        For a single file:
    
        * No component requested, read the whole file.
        * Component requested, optionally read the component efficiently,
          else read the whole file and extract the component.
        * Derived component requested, read whole file or read relevant
          component and derive.
    
        Disassembled Composite:
    
        * The file to read here is the component itself. Formatter only knows
          about this one component file. Should be no component specified
          in the ``read`` call but the `FileDescriptor` will know which
          component this is.
        * A derived component. The file to read is a component but not the
          specified component. The caching needs the component from which
          it's derived.
    
        Raises
        ------
        FormatterNotImplementedError
            Raised if no implementations were found that could read this
            resource.
        """
        # If the file to read is a ZIP file with a fragment requesting
        # a file within the ZIP file, it is no longer possible to use the
        # direct read from URI option and the contents of the Zip file must
        # be extracted.
        uri = self.file_descriptor.location.uri
>       if uri.fragment and uri.unquoted_fragment.startswith("zip-path="):
E       AttributeError: 'FileResourcePath' object has no attribute 'unquoted_fragment'

.../daf/butler/_formatter.py:442: AttributeError
tests/test_formatter.py::ZipFormatterTestCase::test_logs
Stack Traces | 0.015s run time
self = <test_formatter.ZipFormatterTestCase testMethod=test_logs>

    def test_logs(self):
        from lsst.daf.butler.formatters.logs import ButlerLogRecordsFormatter
        from lsst.daf.butler.logging import ButlerLogRecords
    
        formatter = self._make_formatter(
            ButlerLogRecords, ButlerLogRecordsFormatter, "formatter_tests/logs.json"
        )
>       logs = formatter.read()

tests/test_formatter.py:336: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = lsst.daf.butler.formatters.logs.ButlerLogRecordsFormatter(FileDescriptor(Location(None, '.../work/daf_butler/...butler.../tests/data/formatter_tests.zip'), StorageClass('Something', pytype='lsst.daf.butler.logging.ButlerLogRecords')))
component = None, expected_size = -1, cache_manager = None

    def read(
        self,
        component: str | None = None,
        expected_size: int = -1,
        cache_manager: AbstractDatastoreCacheManager | None = None,
    ) -> Any:
        """Read a Dataset.
    
        Parameters
        ----------
        component : `str`, optional
            Component to read from the file. Only used if the `StorageClass`
            for reading differed from the `StorageClass` used to write the
            file.
        expected_size : `int`, optional
            If known, the expected size of the resource to read. This can be
            used for verification or to decide whether to do a direct read or a
            file download. ``-1`` indicates the file size is not known.
        cache_manager : `AbstractDatastoreCacheManager`
            A cache manager to use to allow a formatter to cache a remote file
            locally or read a cached file that is already local.
    
        Returns
        -------
        in_memory_dataset : `object`
            The requested Dataset.
    
        Notes
        -----
        This method should not be subclassed. Instead formatter subclasses
        should re-implement the specific `read_from_*` methods as appropriate.
        Each of these methods has a corresponding class property that must
        be `True` for the method to be called.
    
        The priority for reading is:
    
        * `read_from_uri`
        * `read_from_stream`
        * `read_from_local_file`
        * `read_from_uri` (but with a local file)
    
        Any of these methods can return `NotImplemented` if there is a desire
        to skip to the next one in the list. If a dataset is being requested
        with no component, no parameters, and it should also be added to the
        local cache, the first two calls will be skipped (unless
        `read_from_stream` is the only implemented read method) such that a
        local file will be used.
    
        A Formatter can also read a file from within a Zip file if the
        URI associated with the `FileDescriptor` corresponds to a file with
        a `.zip` extension and a URI fragment of the form
        ``zip-path={path_in_zip}``. When reading a file from within a Zip
        file the priority for reading is:
    
        * `read_from_stream`
        * `read_from_local_file`
        * `read_from_uri`
    
        There are multiple cases that must be handled for reading:
    
        For a single file:
    
        * No component requested, read the whole file.
        * Component requested, optionally read the component efficiently,
          else read the whole file and extract the component.
        * Derived component requested, read whole file or read relevant
          component and derive.
    
        Disassembled Composite:
    
        * The file to read here is the component itself. Formatter only knows
          about this one component file. Should be no component specified
          in the ``read`` call but the `FileDescriptor` will know which
          component this is.
        * A derived component. The file to read is a component but not the
          specified component. The caching needs the component from which
          it's derived.
    
        Raises
        ------
        FormatterNotImplementedError
            Raised if no implementations were found that could read this
            resource.
        """
        # If the file to read is a ZIP file with a fragment requesting
        # a file within the ZIP file, it is no longer possible to use the
        # direct read from URI option and the contents of the Zip file must
        # be extracted.
        uri = self.file_descriptor.location.uri
>       if uri.fragment and uri.unquoted_fragment.startswith("zip-path="):
E       AttributeError: 'FileResourcePath' object has no attribute 'unquoted_fragment'

.../daf/butler/_formatter.py:442: AttributeError

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@timj
Copy link
Member

timj commented Dec 5, 2024

I'm a bit confused with the failures because it's trying to merge into main:

 HEAD is now at cd8c3a50 Merge 6f8970208fc789e7b4bc8ae1ba903fde71f376af into f8303a5aea0268e78ae1e5ceb0ca15252ed2f26e

even though it says v28.0.x. -- the failure is consistent with main being involved.

@mwittgen
Copy link
Contributor Author

mwittgen commented Dec 5, 2024

That could be my fault, when I originally opened the PR it was set to main.

@mwittgen mwittgen closed this Dec 5, 2024
@mwittgen mwittgen reopened this Dec 5, 2024
@mwittgen
Copy link
Contributor Author

mwittgen commented Dec 5, 2024

Let's see what close/reopen the PR does.

@mwittgen
Copy link
Contributor Author

mwittgen commented Dec 5, 2024

@timj
Copy link
Member

timj commented Dec 5, 2024

Aha. That fixed it.

@mwittgen mwittgen merged commit 22d49dd into v28.0.x Dec 6, 2024
27 of 31 checks passed
@mwittgen mwittgen deleted the tickets/DM-47944 branch December 6, 2024 14:55
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.

3 participants