Fixing unhandled corner case in Frequency #700
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
- remove npoints setter - remove f setter - add sweep_type property (sweep_type deduced from f, no more kept from the init)
This was
linked to
issues
Jun 11, 2022
- also fixing resampling using an integer value, error undetected (added tests)
github-actions
bot
added
the
Documentation
|
notebook tests pass (on my fork), so it's should be OK to be merged. Waiting some days for additional comments if any. |
|
Should we deprecate this "feature" first and removing the functionality one release later? |
|
Eventually. While this is more a bad practice than a feature, it's true that some code previously written may stop working...
|
|
I'll made another PR to make a warning |
This was referenced Jun 18, 2022
Merged
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.
The following should avoid the issues #492 and #91: with
Frequency.fproperties andFrequency.npointsimmutable, users should not be able to modify the frequency points.The recommended workflow to change the frequency is to use the resampling/interpolating methods, and not directly manipulating the frequency properties.