WIP: Discussion PR on SpectrumCollections decorator #471
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
|
Am I missing it? I don't see a decorator example in the code? |
|
@nmearl it would probably help if I included the commit, LOL |
|
Oh, another note, @eteq suggested putting the decorator in the SpectrumCollections class as a static method. I'm not sure I'm convinced of this. I feel like it makes the decorator call kind of awkward. |
|
Can you provide an example of using this decorator? |
|
It's in there.... in the resample function: https://github.com/astropy/specutils/pull/471/files#diff-64489eca25a7793c6b98cd48fa905511R125 There's also a test in the test_resample that uses it this way, although it needs to be finished, as right now it's just making sure it runs without crashing. |
SaOgaz
force-pushed
the
collections_decorator
branch
from
32244d2
to
3924fdd
Compare
SaOgaz
force-pushed
the
collections_decorator
branch
from
3924fdd
to
3cd5e08
Compare
|
This PR is stale and author no longer works at STScI. |
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.
Hi all,
@eteq and I had an offline discussion during my work on the resample code about how we want to work in analysis on spectrum collections. I advocated for a decorator. I've done an example of what I had in mind here that runs on my resample class. @eteq had an alternate suggestion that I'm now blanking on.
I like the decorator approach as it seems a little easier to follow for users trying to track what's going on with the code, but this PR is definitely meant for discussion!