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

Adding font Size as default parameter #29217

Closed
wants to merge 17 commits into from

Conversation

saikarna913
Copy link
Contributor

@saikarna913 saikarna913 commented Dec 2, 2024

Adding the font size as default parameter

PR summary

-This PR automatically adjusts the font size when creating the table, provided the fontsize argument is specified, without requiring an explicit call to the set_fontsize function.
-Automatically applying the font size ensures consistent styling during table creation, streamlining the process for users. It eliminates the need for additional method calls

PR checklist

Adding the font size as default parameter
@rcomer
Copy link
Member

rcomer commented Dec 2, 2024

Thank you for your contribution @saikarna913. Please look through the points in our Summary for Authors and try to address any that seem relevant. In particular you will need to add a test and a type hint. After adding the type hint, you will need to run boilerplate.py so that pyplot.table is updated to match Axes.table.

@saikarna913
Copy link
Contributor Author

saikarna913 commented Dec 2, 2024

@rcomer i want to test this code, but I am unable to install the matplotlib completely it is giving me the import error
I’m facing issues installing Matplotlib in the development environment, both on my local desktop and in GitHub Codespaces. It’s throwing an import error, so I’m unable to test the code changes I’ve made.

Here’s what I’ve done so far:

  1. Forked the Matplotlib repository.
  2. Created a new environment and installed dependencies using:
    pip install -r requirements/dev/dev-requirements.txt
  3. also tried GitHub Codespaces by activating the environment with:
    conda activate mpl-dev
  4. Installed Matplotlib in editable mode with:
    python -m pip install --verbose --no-build-isolation --editable ".[dev]"
  5. Also tried:
    python -m pip install --no-build-isolation --config-settings=editable-verbose=true --editable .

Despite these steps, I’m still encountering the import error. Could you help me figure out why this might be happening?

@rcomer
Copy link
Member

rcomer commented Dec 2, 2024

I just tried running the tests against a clean branch in codespaces. I saw a few failures (#29218) but no ImportError, so I am not sure what to suggest.

@saikarna913
Copy link
Contributor Author

@rcomer I have tested the code by using this test code

import matplotlib.pyplot as plt
import numpy as np

# Data for plotting
x = np.linspace(0, 10, 100)
y = x + 1
tableData = [['a', 1], ['b', 1]]

# Create the figure and axis objects
fig, ax = plt.subplots()

# Plot the data
ax.plot(x, y)

# Add a table
t = ax.table(
    cellText=tableData,  # Table content
    loc='top',  # Position of the table
    cellLoc='center',  # Center-align the text in cells
    fontsize=30  # Set the font size for the table
)

# Save the figure as an image
plt.savefig('output_plot_with_table_after.png')  # Save as PNG file

# Optionally, close the plot (no need to display it)
plt.close()

I got this output
output_plot_with_table_after

As a beginner, I’m not fully sure about type hints. Could you guide me on where exactly to add them in my code? I also attempted to include the following test code:

import pytest
import matplotlib.pyplot as plt
import numpy as np

# Test function for fontsize in table
def test_table_fontsize():
    # Data for plotting
    tableData = [['a', 1], ['b', 1]]
    
    # Create the figure and axis objects
    fig, ax = plt.subplots()
    
    # Plot the data (although we are focusing on the table)
    ax.plot(np.linspace(0, 10, 100), np.linspace(0, 10, 100) + 1)
    
    # Add a table with fontsize=30
    t = ax.table(
        cellText=tableData, 
        loc='top', 
        cellLoc='center', 
        fontsize=30
    )
    
    # Retrieve the font size from a specific cell (e.g., cell (0, 0))
    cell_fontsize = t[(0, 0)].get_fontsize()
    
    # Assert that the fontsize is correctly set to 30
    assert cell_fontsize == 30, f"Expected fontsize 30, but got {cell_fontsize}"

    # Clean up by closing the plot (important for pytest to avoid hanging processes)
    plt.close()

This test code has been running for a long time. I don't know why.

@rcomer
Copy link
Member

rcomer commented Dec 2, 2024

The type hint goes in table.pyi. Hopefully when you compare what's already in there with table.py it should be fairly intuitive what needs adding.

@rcomer rcomer linked an issue Dec 2, 2024 that may be closed by this pull request
@saikarna913
Copy link
Contributor Author

@rcomer , please review this PR and approve it

  • added the type hint and ran the boilerplate
  • tested the code with a new test case added in test_table.py

@NGWi
Copy link

NGWi commented Dec 8, 2024

Thank you, @saikarna913.
This may sound annoying, but can you correct the flake8 formatting issues? If you work in VS Code, there are extensions to highlight flake8 issues.

@saikarna913
Copy link
Contributor Author

@NGWi removed the flake issues
can you approve this pr

@rcomer
Copy link
Member

rcomer commented Dec 8, 2024

@saikarna913 it looks like you have run an auto-formatter such as black. This makes the size of the change much larger than is necessary for the PR. I’m sorry but you will need to revert those changes. Note that Matplotlib intentionally does not use black.

@saikarna913 saikarna913 force-pushed the main branch 2 times, most recently from e330e1e to b81cf6c Compare December 8, 2024 17:07
@saikarna913
Copy link
Contributor Author

saikarna913 commented Dec 8, 2024

@rcomer done. please let me know if any changes required

@rcomer
Copy link
Member

rcomer commented Dec 8, 2024

@saikarna913 please take a look at the automated tests below. Mypy has failed although it was passing before. Mypy checks the type hints so something has gone wrong there.

@rcomer
Copy link
Member

rcomer commented Dec 8, 2024

Actually the Mypy failure is something to do with pylab, which you have not changed. So I don't actually know what is going on there 😕

@saikarna913 saikarna913 closed this Dec 8, 2024
@rcomer
Copy link
Member

rcomer commented Dec 8, 2024

@saikarna913 did you mean to close this?

@QuLogic
Copy link
Member

QuLogic commented Dec 10, 2024

I believe this has been replaced by #29258.

@saikarna913
Copy link
Contributor Author

saikarna913 commented Dec 10, 2024

I believe this has been replaced by #29258.

yes, can you review my changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: fontsize in tables not working
4 participants