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

Restructure package #43

Merged
merged 9 commits into from
Feb 14, 2025
Merged

Restructure package #43

merged 9 commits into from
Feb 14, 2025

Conversation

mberz
Copy link
Member

@mberz mberz commented Dec 23, 2024

  • Do not import content of each sub-module to top level
  • Rename sub-modules if required

Tests will be adapted as soon as the import structure is finalized.
Functions which have pyfar contour parts already will be replaced in separate PRs

@mberz mberz added this to the v1.0.0 milestone Jan 3, 2025
Copy link
Member

@ahms5 ahms5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

In pyfar we have a folder for each submodule, with a init as well. In this way its easier to manage what is public, and what not, and whats part of the doc and what not. I would suggest to shift the submodules, into folders with a Init defining which method should be used and which not (e.g. private helpers).

pyrato/__init__.py Outdated Show resolved Hide resolved
pyrato/parametric.py Show resolved Hide resolved
@mberz mberz changed the title rewrite init based on current files Restructure package Jan 13, 2025
@mberz
Copy link
Member Author

mberz commented Jan 13, 2025

Thank you!

In pyfar we have a folder for each submodule, with a init as well. In this way it's easier to manage what is public, and what not, and whats part of the doc and what not. I would suggest to shift the submodules, into folders with an Init defining which method should be used and which not (e.g. private helpers).

I would not do this before agreeing on the new import structure.

@sikersten
Copy link
Member

  • What about giving rap.py a descriptive name?
  • Is it intended that docs are not built correctly, yet?

@mberz
Copy link
Member Author

mberz commented Feb 7, 2025

  • What about giving rap.py a descriptive name?

Any suggestions?

  • Is it intended that docs are not built correctly, yet?

Yes, docs and tests need to be adjusted

@f-brinkmann
Copy link
Member

What about giving rap.py a descriptive name?

Any suggestions?
How about 'parameters'?

What is the idea for later having convenience functions for that, which will wrap truncation, filtering, edc computation and analysis. Would they go in an extra module?

@ahms5 ahms5 added the v1.0.0 label Feb 11, 2025
pyrato/__init__.py Show resolved Hide resolved
pyrato/__init__.py Show resolved Hide resolved
pyrato/__init__.py Show resolved Hide resolved
pyrato/parametric.py Show resolved Hide resolved
pyrato/parametric.py Show resolved Hide resolved
pyrato/rap.py Show resolved Hide resolved
Copy link
Member

@ahms5 ahms5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and we need to merge dev_v1.0.0 again, since there where changes on main, which were not merged into dev

@mberz
Copy link
Member Author

mberz commented Feb 12, 2025

What about giving rap.py a descriptive name?

Any suggestions?
How about 'parameters'?

What is the idea for later having convenience functions for that, which will wrap truncation, filtering, edc computation and analysis. Would they go in an extra module?

I would argue that parameters is even less descriptive than (r)oom (a)coustic (p)arameters ;)
I'm not sure about the convenience functions. I have nothing against placing them in here as well, seems like a fitting place.

@f-brinkmann
Copy link
Member

I'm fine with both names, rap and paramters.

@mberz
Copy link
Member Author

mberz commented Feb 13, 2025

and we need to merge dev_v1.0.0 again, since there where changes on main, which were not merged into dev

done

@mberz mberz merged commit 7d9040a into develop_1.0.0 Feb 14, 2025
1 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants