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

replace Deprecated method merged_cell_ranges #24

Closed
wants to merge 3 commits into from
Closed

replace Deprecated method merged_cell_ranges #24

wants to merge 3 commits into from

Conversation

PaleNeutron
Copy link

merged_cell_ranges is replaced by merged_cells.ranges
link: http://openpyxl.readthedocs.io/en/default/_modules/openpyxl/worksheet/worksheet.html

I my opinion `MergedCell` class here should be removed and use `MultiCellRange` in `openpyxl` instead.
@chfw
Copy link
Member

chfw commented Feb 19, 2018

Thanks for your contribution. Here are the issues:

  1. Can you update the unit tests? https://github.com/pyexcel/pyexcel-xlsx/blob/master/tests/test_merged_cells.py#L75

  2. Please update openpyxl requirements

  3. Please remove python 3.3 test target from .travis.yml

@PaleNeutron
Copy link
Author

PaleNeutron commented Feb 20, 2018

In my opinion, current MergedCell is duplicated with openpyxl's CellRange

For eq.

    @property
    def bounds(self):
        """
        Vertices of the range as a tuple
        """
        return self.min_col, self.min_row, self.max_col, self.max_row

This property shows the CellRange class can offer all features MergedCell have.

Should MergedCell be completely removed or keep it and just fix the unit-test?

@chfw
Copy link
Member

chfw commented Mar 15, 2018

@PaleNeutron , I agree with you that MergedCell should be removed.

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.

None yet

2 participants