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

fix: improve description and correct plot for ‘auto’ correlation #1119

Merged
merged 3 commits into from
Oct 29, 2022

Conversation

jtook
Copy link
Contributor

@jtook jtook commented Oct 20, 2022

No description provided.

@jtook jtook force-pushed the feat/correlation_auto_visual_update branch from 3fef9e8 to cb8b8c0 Compare October 20, 2022 15:17
@vascoalramos vascoalramos force-pushed the feat/correlation_auto_visual_update branch 2 times, most recently from a628bed to 629d97c Compare October 21, 2022 15:28
@aquemy
Copy link
Contributor

aquemy commented Oct 24, 2022

@fabclmnt the other examples are in python while this one is a Notebook. I am OK with that, and the example if fine to me but I just want to check with you that we can allow Notebooks as examples in the future.

Copy link
Contributor

@fabclmnt fabclmnt left a comment

Choose a reason for hiding this comment

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

@jtook this is not a feat it is at maximum a fix because you included changes at the level of the correlation.py file.

As examples are out of the package they should not be considered for version increment of the package. Can you please change the PR message in accordance?
Also, can you please confirm the license of the dataset used?

@aquemy as this is for the feature folder and not for the use-cases folder, I'll ask to have this changed to a python file. The change is minor and it eases the consumption of the code to generate automated documentation, as we have in other examples.

Notebooks should be used for examples such as usaairquality or employees-chicago.

@jtook jtook force-pushed the feat/correlation_auto_visual_update branch from 14d8d3c to 09aab0b Compare October 28, 2022 11:15
@jtook jtook force-pushed the feat/correlation_auto_visual_update branch from 09aab0b to b698292 Compare October 28, 2022 11:19
@jtook jtook force-pushed the feat/correlation_auto_visual_update branch from b698292 to db85ab7 Compare October 28, 2022 15:03
@jtook jtook changed the title feat: improve description and correct plot for ‘auto’ correlation fix: improve description and correct plot for ‘auto’ correlation Oct 28, 2022
@codecov-commenter
Copy link

Codecov Report

Base: 90.95% // Head: 90.95% // No change to project coverage 👍

Coverage data is based on head (dc97941) compared to base (06ec172).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1119   +/-   ##
========================================
  Coverage    90.95%   90.95%           
========================================
  Files          179      179           
  Lines         5084     5084           
========================================
  Hits          4624     4624           
  Misses         460      460           
Flag Coverage Δ
py3.8-ubuntu-latest-pandas 90.95% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../pandas_profiling/report/structure/correlations.py 96.42% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jtook jtook requested review from aquemy and fabclmnt October 28, 2022 16:15
@jtook
Copy link
Contributor Author

jtook commented Oct 28, 2022

The changes you requested have been made. I used a dataset that is already being used in the examples folder from the UCI dataset collection

@aquemy aquemy dismissed fabclmnt’s stale review October 29, 2022 15:28

I verified the changes with James

@aquemy aquemy merged commit 3ad4f0d into develop Oct 29, 2022
@aquemy aquemy deleted the feat/correlation_auto_visual_update branch October 29, 2022 15:29
vascoalramos pushed a commit that referenced this pull request Nov 22, 2022
* fix: correct plot and formatting for ‘auto’ correlation

* fix: create example ‘auto’ correlation notebook

* fix: add example for 'auto' correlation in python script
vascoalramos pushed a commit that referenced this pull request Nov 22, 2022
* fix: correct plot and formatting for ‘auto’ correlation

* fix: create example ‘auto’ correlation notebook

* fix: add example for 'auto' correlation in python script
aquemy pushed a commit that referenced this pull request Nov 22, 2022
* fix: correct plot and formatting for ‘auto’ correlation

* fix: create example ‘auto’ correlation notebook

* fix: add example for 'auto' correlation in python script
aquemy pushed a commit that referenced this pull request Jan 30, 2023
* fix: correct plot and formatting for ‘auto’ correlation

* fix: create example ‘auto’ correlation notebook

* fix: add example for 'auto' correlation in python script
aquemy pushed a commit that referenced this pull request Jan 30, 2023
* fix: correct plot and formatting for ‘auto’ correlation

* fix: create example ‘auto’ correlation notebook

* fix: add example for 'auto' correlation in python script
vascoalramos pushed a commit that referenced this pull request Jan 30, 2023
* fix: correct plot and formatting for ‘auto’ correlation

* fix: create example ‘auto’ correlation notebook

* fix: add example for 'auto' correlation in python script
vascoalramos pushed a commit that referenced this pull request Jan 30, 2023
* fix: correct plot and formatting for ‘auto’ correlation

* fix: create example ‘auto’ correlation notebook

* fix: add example for 'auto' correlation in python script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants