Conversation
Using `@dataclass` vastly simplifies the implementation of `Detector`, and the new name `DetectorInfo` makes the connection with the IMO quantity clearer.
Apparently, Poetry 1.0 does not handle dependencies on specific versions of Python correctly: python-poetry/poetry#1413 While waiting for Poetry 1.1, this commit tries to fix the problem.
This is a **breaking** change: the `Instrument` class is renamed to `InstrumentInfo`, to match the names `DetectorInfo` and `FreqChannelInfo`. For consistency, the class has been moved from `instruments.py` to `detectors.py`, which already contained the definition for classes `DetectorInfo` and `FreqChannelInfo`. Moreover, the fields used by `InstrumentInfo` to hold angles can no longer be initialized with values in *degrees*: you must explicitly pass them as radians. This makes the interface to the class clearer: the parameters you pass to the constructor are the same you can access once the object has been constructed. The mock IMO used in tests has been updated to use the name `instrument_info` instead of `focal_plane`, to be consistent with the true IMO. A few new data files have been added to this IMO, to test the ability to produce data files. All the examples in the documentation have been updated accordingly.
This was missing from the previous commit. It implements a general-purpose way to produce a list of `DetectorInfo` objects out of a set of definitions in a dictionary, and it is meant to be used with TOML files. A new page about detectors, frequency channels, and instruments has been added to the documentation.
This might resolve a few problems when installing the "pillow" dependency under Mac OS X: python-pillow/Pillow#4242 (the title of the issue is misleading, as the problem does not affect only Windows user, as it's evident from the thread.)
This breaking change is motivated by issue #65 (#65). It changes the name and meaning of the parameters to `SpinningScanningStrategy.__init__` in the following way: - `spin_sun_angle_deg` -> `spin_sun_angle_rad` (from degrees to radians) - `spin_rate_rpm` -> `spin_rate_hz` (from RPM to Hertz) - `precession_period_min` -> `precession_rate_hz` (from a time expressed in minutes to a frequency in Hz) The reason of this is that in this way there is no unit conversion in the constructor, which leads to a more predictable behavior of the class. The reason why the internal fields have stayed the same is because in their current form they ensure the fastest code (i.e., no conversions during calculations).
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This breaking change is motivated by issue #65. It changes the name and meaning of the parameters to
SpinningScanningStrategy.__init__in the following way:spin_sun_angle_deg→spin_sun_angle_rad(from degrees to radians)spin_rate_rpm→spin_rate_hz(from RPM to Hz)precession_period_min→precession_rate_hz(from a time expressed in minutes to a frequency in Hz)The reason of this is that in this way there is no unit conversion in the constructor, which leads to a more predictable behavior of the class. The reason why the internal fields have stayed the same is because in their current form they ensure the fastest code (i.e., no conversions during calculations).
As the IMO lists these parameters using the old units (degrees, rpm, minutes), the
SpinningScanningStrategy.from_imomethods performs the conversion automatically, instead of relying on the constructor.