You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "Jualhosting (via GitHub)" <gi...@apache.org> on 2026/05/27 05:56:11 UTC

[PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Jualhosting opened a new pull request, #40456:
URL: https://github.com/apache/superset/pull/40456

   Fixes #36530 by explicitly copying the dataframe after dropna in histogram postprocessing to avoid SettingWithCopyWarning
   
   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [x] Has associated issue: #36530
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   * closes: #36530


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #40456:
URL: https://github.com/apache/superset/pull/40456#issuecomment-4605614842

   <!-- Bito Reply -->
   The observation is correct. The existing tests in `test_histogram.py` do not trigger the `SettingWithCopyWarning` because they use newly constructed DataFrames, which do not have the `_is_copy` attribute set. Adding a test case that uses a sliced DataFrame, as suggested, is necessary to verify that the warning is effectively suppressed in the intended scenarios.
   
   **superset/utils/pandas_postprocessing/histogram.py**
   ```
   def test_histogram_no_setting_with_copy_warning():
       import warnings
       from pandas.errors import SettingWithCopyWarning
       source = DataFrame({"a": [1, 2, None, 4, 5], "b": list(range(5))})
       with warnings.catch_warnings(record=True) as w:
           warnings.simplefilter("always")
           histogram(source.loc[source["b"] >= 0], "a", [], 3)
       assert not any(issubclass(x.category, SettingWithCopyWarning) for x in w)
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #40456:
URL: https://github.com/apache/superset/pull/40456#issuecomment-5647700949

   Quiet since June, and the conflict's only grown since. As @aminghadersohi's dig turned up, the `.copy()` here is probably moot now that #36605 already fixed the actual warning site, and the only thing failing CI is the import-order nit from Joe's original note. Rather than let this sit any longer, I'll just rebase and clean that up myself. Pushing shortly.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on code in PR #40456:
URL: https://github.com/apache/superset/pull/40456#discussion_r3343549193


##########
superset/utils/pandas_postprocessing/histogram.py:
##########
@@ -49,7 +49,7 @@ def histogram(
         groupby = []
 
     # drop empty values from the target column
-    df = df.dropna(subset=[column])
+    df = df.dropna(subset=[column]).copy()

Review Comment:
   Can we add some regression tests around this such that in the future no one removes the `.copy()`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #40456:
URL: https://github.com/apache/superset/pull/40456#issuecomment-6114124004

   Checking back in on this one, and thanks for the fix. I think the situation has changed since you opened it.
   
   #36605 moved the write in `histogram.py` to `df.loc[:, column] = ...`, and that no longer trips `SettingWithCopyWarning`. @aminghadersohi verified on the current head that even a real sliced fixture passes with `.copy()` removed, so there's no test we can write that would catch a regression here. That also means the thing @sadpandajoe asked for can't be done anymore, and #36530 looks fixed on master already.
   
   Tempted to close this as superseded, but curious whether you're still seeing the warning on a recent master. If you are, a repro would be great and we can take another look.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #40456:
URL: https://github.com/apache/superset/pull/40456#issuecomment-4551789650

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #40456:
URL: https://github.com/apache/superset/pull/40456#issuecomment-4684922287

   <!-- translation-regression-bot -->
   ## ⚠️ Translation Regression Detected
   
   This PR causes existing translations to become fuzzy or be removed in `ro`. Please fix the affected `.po` files before merging.
   
   | Language | Before | After | Lost |
   |----------|-------:|------:|-----:|
   | `ro` | 4343 | 0 | -4343 |
   
   ### How to fix
   
   **1. Install dependencies** (if not already set up):
   
   ```bash
   pip install -r superset/translations/requirements.txt
   sudo apt-get install gettext   # or: brew install gettext
   ```
   
   **2. Re-extract strings and sync `.po` files:**
   
   ```bash
   ./scripts/translations/babel_update.sh
   ```
   
   This rewrites `superset/translations/messages.pot` from the current source files and merges the changes into every `.po` file. Strings whose `msgid` changed will be marked `#, fuzzy`.
   
   **3. Resolve the fuzzy entries** in the affected language files (`ro`):
   
   ```bash
   grep -n '#, fuzzy' superset/translations/<lang>/LC_MESSAGES/messages.po
   ```
   
   For each fuzzy entry, either rewrite the `msgstr` to match the new string and remove the `#, fuzzy` line, or clear the `msgstr` to `""` if you cannot provide a translation.
   
   **4. Commit your changes to the `.po` files.**
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #40456:
URL: https://github.com/apache/superset/pull/40456#issuecomment-4940426721

   The `.copy()` fix itself is right. Joe's point is valid though that the test should use an actual sliced frame that fails without the `.copy()`, otherwise it doesn't really guard against someone removing it later. CI is also red at the moment. Let me know if the sliced-fixture setup is fiddly, or if I/we can help.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #40456:
URL: https://github.com/apache/superset/pull/40456#issuecomment-4610136687

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.

aminghadersohi commented on code in PR #40456:
URL: https://github.com/apache/superset/pull/40456#discussion_r3343395699


##########
superset/utils/pandas_postprocessing/histogram.py:
##########
@@ -49,7 +49,7 @@ def histogram(
         groupby = []
 
     # drop empty values from the target column
-    df = df.dropna(subset=[column])
+    df = df.dropna(subset=[column]).copy()
     if df.empty:

Review Comment:
   The fix is correct, but the PR description says "Unit tests verify the warning is no longer raised" — that claim is not accurate. `test_histogram.py` has 10 existing behavioral tests, none of which assert on `SettingWithCopyWarning` absence. The existing tests use freshly-constructed DataFrames that won't trigger the warning regardless of this change, because `_is_copy` is `None` on directly-constructed DataFrames.
   
   The actual triggering path is passing a slice: `histogram(df[df["date"] == x], col, [], bins)`. Please add a test that covers that input shape, e.g.:
   
   ```python
   def test_histogram_no_setting_with_copy_warning():
       import warnings
       from pandas.errors import SettingWithCopyWarning
       source = DataFrame({"a": [1, 2, None, 4, 5], "b": list(range(5))})
       with warnings.catch_warnings(record=True) as w:
           warnings.simplefilter("always")
           histogram(source.loc[source["b"] >= 0], "a", [], 3)
       assert not any(issubclass(x.category, SettingWithCopyWarning) for x in w)
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #40456:
URL: https://github.com/apache/superset/pull/40456#issuecomment-5649240808

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers an incremental AI Review.
   - `/review full` - Manually triggers a full AI Review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.

aminghadersohi commented on code in PR #40456:
URL: https://github.com/apache/superset/pull/40456#discussion_r3357133995


##########
tests/unit_tests/pandas_postprocessing/test_histogram.py:
##########
@@ -207,3 +207,13 @@ def test_histogram_with_no_groupby_and_all_null_values():
 
     result = histogram(data_with_no_groupby_and_all_nulls, "a", [], bins)
     assert result.empty
+
+
+def test_histogram_no_setting_with_copy_warning():
+    import warnings

Review Comment:
   Move `import warnings` and `from pandas.errors import SettingWithCopyWarning` to module-level imports.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.

netlify[bot] commented on PR #40456:
URL: https://github.com/apache/superset/pull/40456#issuecomment-4609134039

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | 69a995f99739970d7cc8a2aefebb138e94ef8f34 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a1fafec25e3a70008e3dbef |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-40456--superset-docs-preview.netlify.app](https://deploy-preview-40456--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQwNDU2LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.Ag7r3O11S-_knA3CE37QIPK1P9ierk3i9JjMIuFB65Q)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#q1YqKMrPLShRslJS0lFKKkrMS84AstMyK3QzMotL8tOLEnN1yxOL8jLz0oEKUlILcvIrPVOASswSDdMS01KTjUxTjRPNDQwMLFKNU5JS05RqAQ) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-40456--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas closed pull request #40456: fix: avoid SettingWithCopyWarning in pandas histogram postprocessing
URL: https://github.com/apache/superset/pull/40456


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on code in PR #40456:
URL: https://github.com/apache/superset/pull/40456#discussion_r3462437924


##########
tests/unit_tests/pandas_postprocessing/test_histogram.py:
##########
@@ -207,3 +207,13 @@ def test_histogram_with_no_groupby_and_all_null_values():
 
     result = histogram(data_with_no_groupby_and_all_nulls, "a", [], bins)
     assert result.empty
+
+
+def test_histogram_no_setting_with_copy_warning():
+    import warnings
+    from pandas.errors import SettingWithCopyWarning
+    source = DataFrame({"a": [1, 2, None, 4, 5], "b": list(range(5))})
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter("always")
+        histogram(source.loc[source["b"] >= 0], "a", [], 3)

Review Comment:
   I believe this will still work even if you removed `.copy()`. Should verify this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on code in PR #40456:
URL: https://github.com/apache/superset/pull/40456#discussion_r3462473762


##########
tests/unit_tests/pandas_postprocessing/test_histogram.py:
##########
@@ -207,3 +207,13 @@ def test_histogram_with_no_groupby_and_all_null_values():
 
     result = histogram(data_with_no_groupby_and_all_nulls, "a", [], bins)
     assert result.empty
+
+
+def test_histogram_no_setting_with_copy_warning():
+    import warnings
+    from pandas.errors import SettingWithCopyWarning
+    source = DataFrame({"a": [1, 2, None, 4, 5], "b": list(range(5))})
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter("always")
+        histogram(source.loc[source["b"] >= 0], "a", [], 3)

Review Comment:
   I think this needs to prove the regression by using an actual sliced input. Since `source["b"] >= 0` selects every row, this doesn’t really exercise the slice/view case that would warn if the `.copy()` in `histogram.py` were removed. Can we make the fixture a real subset and verify locally that this test fails when `.copy()` is temporarily removed?
   
   ```suggestion
       source = DataFrame({"a": [1, 2, None, 4, 5], "b": list(range(5))})
       sliced = source[source["b"] > 0]
       with warnings.catch_warnings(record=True) as w:
           warnings.simplefilter("always")
           histogram(sliced, "a", [], 3)
   
   ```suggestion
       source = DataFrame({"a": [1, 2, None, 4, 5], "b": list(range(5))})
       sliced = source[source["b"] >= 0]
       with warnings.catch_warnings(record=True) as w:
           warnings.simplefilter("always")
           histogram(sliced, "a", [], 3)
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.

codecov[bot] commented on PR #40456:
URL: https://github.com/apache/superset/pull/40456#issuecomment-4605722995

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40456?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `0%` with `1 line` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 55.36%. Comparing base ([`09a94fa`](https://app.codecov.io/gh/apache/superset/commit/09a94fa26b4c0aa1bc97629031de3cd9077caf23?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0aaafb3`](https://app.codecov.io/gh/apache/superset/commit/0aaafb3df7219228d7657de86cda042e6cbccfd4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 122 commits behind head on master.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/40456?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [superset/utils/pandas\_postprocessing/histogram.py](https://app.codecov.io/gh/apache/superset/pull/40456?src=pr&el=tree&filepath=superset%2Futils%2Fpandas_postprocessing%2Fhistogram.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nL2hpc3RvZ3JhbS5weQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40456?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   > :exclamation:  There is a different number of reports uploaded between BASE (09a94fa) and HEAD (0aaafb3). Click for more details.
   > 
   > <details><summary>HEAD has 206 uploads less than BASE</summary>
   >
   >| Flag | BASE (09a94fa) | HEAD (0aaafb3) |
   >|------|------|------|
   >|python|175|2|
   >|presto|17|1|
   >|hive|18|1|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #40456      +/-   ##
   ==========================================
   - Coverage   64.18%   55.36%   -8.82%     
   ==========================================
     Files        2592     2592              
     Lines      139271   139271              
     Branches    32337    32337              
   ==========================================
   - Hits        89385    77113   -12272     
   - Misses      48351    61492   +13141     
   + Partials     1535      666     -869     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/40456/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/40456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.21% <0.00%> (ø)` | |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/40456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/40456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/40456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.89% <0.00%> (ø)` | |
   | [python](https://app.codecov.io/gh/apache/superset/pull/40456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.96% <0.00%> (-19.40%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/40456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/40456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/40456?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: avoid SettingWithCopyWarning in pandas histogram postprocessing [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.

aminghadersohi commented on code in PR #40456:
URL: https://github.com/apache/superset/pull/40456#discussion_r4002426927


##########
tests/unit_tests/pandas_postprocessing/test_histogram.py:
##########
@@ -207,3 +207,13 @@ def test_histogram_with_no_groupby_and_all_null_values():
 
     result = histogram(data_with_no_groupby_and_all_nulls, "a", [], bins)
     assert result.empty
+
+
+def test_histogram_no_setting_with_copy_warning():
+    import warnings
+    from pandas.errors import SettingWithCopyWarning
+    source = DataFrame({"a": [1, 2, None, 4, 5], "b": list(range(5))})
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter("always")
+        histogram(source.loc[source["b"] >= 0], "a", [], 3)

Review Comment:
   Verified at this head: your suggested fixture (`sliced = source[source["b"] > 0]`) still passes with `.copy()` removed. #36605 moved the write site to `df.loc[:, column] =`, which no longer trips the check, so no fixture can make this test fail without `.copy()`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org