-
Notifications
You must be signed in to change notification settings - Fork 3
Feature/joss review comments #13
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
Open
nielsschlusser
wants to merge
33
commits into
main
Choose a base branch
from
feature/JOSS_review_comments
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
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
- implement explained_variance_ratio_ for both, NipalsPCA and NipalsPLS - implemented corresponding unit tests - re-organized imports to comply with pylint policy - currently still empty slice warning in NipalsPLS l. 799
- set up documentation with sphinx-quickstart - built documentation with tox -e docs - view docs with python -m http.serve from docs/ dir and visiting localhost:8000 in browser
- create docs with sphinx - adapted docstrings in src/open_nipals/ folder
- include .readthedocs.yaml config file - change README.md pypi installation paragraph - recompile the docs
Change to python version 3.13
Address rsenne's Small patch candidate 1.
- guard convergence division against zero norm in both modules, adressing rsenne's small patch candidates 2. - restructure calc_imd in nipalsPCA.py in line with rsenne's small patch candidate suggestion 1
Use pseudo-inverse in nipalsPCA.py projection bit in transform to address rsenne's small patch candidates 3.
- change print to warning statement in case iterations are terminated due to reaching maximum number of iterations in both modules, addressing rsenne's small patch candidate 4. - use fstrings in the prints for verbose option instead of .format(...)
- use np.concatenate instead of np.append for performance gains, addressing rsenne's small patch candidates 6 - relax the default relative tolerance criteria to 1e-6, addressing rsenne's comment on performance
Correct bug in NIpalsPLS.calc_imd when using scores
Missed another inv->pinv
unify exponential numbers format
- change base classes for NipalsPCA from _BasePCA (private, unstable) to BaseEstimator, TransformerMixin (public) - change base classes for NipalsPLS from _PLS (private, unstable) to BaseEstimator, TransformerMixin, RegressorMixin (public) - re-organize inputs to comply with pylint input organization hierarchy (python native, thrid party, first party)
- fit new components in NipalsPCA transform conditionalMean with set_components, adddressing rsenne's major issue 5 - updated and streamlined documentation where necessary - streamlined NipalsPLS calc_oomd, adapted to NipalsPCA counterpart - re-create documentation
- Ledoit Wolf does still not work due to dimensionality issue - updated docs
- two more options for calc_imd in both modules: full using the full covariance matrix, and ledoit_wolf for using the full covariance matrix using the Ledoit Wolf shrinkage for performance - update the docs
- split get_explained_variance and get_Explained_variance_ratio methods from properties - both methods now return numpy arrays - fixed the unit tests
This was referenced Jan 6, 2026
- nipalsPCA.py: remove use_Denom=True statements, as this is the default value anyway - nipalsPLS.py: follow the recipe of K. Dunn's book for normalization - update the documentation
Correct normalization
This was referenced Jan 8, 2026
- implement tighter limits on PLS unit tests - remove regression vector computation from set_Components unit test
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.
This feature branch contains the adjustments and augmentations of the code based on @rsenne 's suggestions in issues #10 and #12.