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.

Sign up for GitHub

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

Jump to bottom

Bug: explicit casting to DataFrame forces incorrect column names #319

Open
shntnu opened this issue Sep 9, 2023 · 1 comment
Open

Bug: explicit casting to DataFrame forces incorrect column names #319

shntnu opened this issue Sep 9, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@shntnu
Copy link
Member

shntnu commented Sep 9, 2023

Example code with output

In normalize.normalize, this chunk explicitly recreates a pd.DataFrame from feature_df

    feature_df = pd.DataFrame(
        fitted_scaler.transform(feature_df),
        columns=feature_df.columns,
        index=feature_df.index,
    )

This fails when the fitted_scaler.transform(feature_df).columns are different from feature_df.columns

Issue description

For now, I have a temporary fix by doing this

    feature_df = pd.DataFrame(
        fitted_scaler.transform(feature_df).values,
        columns=feature_df.columns,
        index=feature_df.index,
    )

because .values extracts an ndarray, and that works fine (because the column names are removed when converting to ndarray)

Expected behavior

If the scaler is Spherize and method is PCA/PCA-cor, the column names will not be the same as the original dataframe, so we should adapt this appropriately

Additional information

No response
@shntnu shntnu added the bug Something isn't working label Sep 9, 2023
shntnu added a commit to shntnu/pycytominer that referenced this issue Sep 9, 2023
@shntnu
https://github.com/cytomining/pycytominer/issues/319
7ea8197
gwaybio pushed a commit that referenced this issue Jan 26, 2024
@shntnu
Fix Spherize – use SVD, simplify calculations (#320)
8541374 
* #319

* Rewrite spherize

* revert

* black

* skip test

* comment

* docs

* Handle types

* flag zero var

* skip some conditions

* typo

* black

* typo

* checks

* Handle low rank

* fix size check

* prompt user to report bug

* change assertions to exceptions

* Use return_numpy in Spherize

* Fix typo

* handle change of column names

* add test

* test centering

* change assertions to exceptions, and explain better

* clean up error messages, remove requirement for ZCA

* Drop `create_bug_report_message`

* Update error message

* Math clarifications

* Clarify why we don't use np.linalg.inv(S)

* black missed this
@kenibrewer
Copy link
Member

@shntnu Was this issue addressed as part of the previously merged PR? If so, we should close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@shntnu @kenibrewer