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

move resamplers out of parsers? #138

Closed
Tracked by #143
sbillinge opened this issue Oct 26, 2024 · 3 comments · Fixed by #164
Closed
Tracked by #143

move resamplers out of parsers? #138

sbillinge opened this issue Oct 26, 2024 · 3 comments · Fixed by #164
Milestone

Comments

@sbillinge
Copy link
Contributor

Problem

Why are resamplers in the parsers sub-module, they are not parsers?

Proposed solution

create a tools sub-package, or simply put the resamplers into tools.py? Let's discuss what makes the most sense. @Sparks29032 ?

@alisnwu
Copy link

alisnwu commented Nov 6, 2024

@sbillinge @Sparks29032
tools.py seems to only deal with user and package related functions. The documentation also has resampler in its own section, so should we just create a resampler sub-package? It would be diffpy.utils.resampler.wsinterp

Or instead we could do diffpy.utils.tools.interpolation.wsinterp

@Sparks29032
Copy link
Collaborator

Sparks29032 commented Nov 6, 2024

I made a note in #145. I agree the import from diffpy.utils.resample import wsinterp (path diffpy.utils.resample.wsinterp) would be best. Right now the path is from diffpy.utils.parsers.resample import wsinterp.

This means we should pull the reample function out of parsers. However, we should check our codebase to see where resample is used.

@bobleesj
Copy link
Contributor

bobleesj commented Nov 6, 2024

@Sparks29032 from diffpy.utils.resample import wsinterp yeah, sounds good! @alisnwu let's proceed with this: utils/resample.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment