search

clawpack / visclaw

Clawpack visualization tools
http://www.clawpack.org
BSD 3-Clause "New" or "Revised" License
29 stars 48 forks source link

Revert "2D Tiff plot option" #283

Closed rjleveque closed 3 years ago

rjleveque commented 3 years ago

Reverts clawpack/visclaw#282

@AsianHam: Sorry, I merged #282 without properly testing it and it has at least a couple problems that render it inoperable on some standard examples.

In frametools.py:

import geoCon as gc should be from clawpack.visclaw import geoCon as gc

Line 183-184:

            if plotaxes.plotitem_dict['ITEM1'].plot_type == '2d_gtiff':
                gc.render(framesolns[0], frameno)

This is hardwired with the assumption that plotitem_dict has a key ITEM1. But if the user calls plotaxes.new_plotitem with the name argument then the default ITEM1 is not used.

Moreover, I don't understand why this call is made in the loop on plotfigure._axesnames rather than later in the embedded loop on plotaxes._itemnames that starts at line 320. Why is this done only for the first item?

But it's not clear to me what the use case is for this, so perhaps the original PR can be fixed up and a new PR issued with more explanation?

Thanks!

AsianHam commented 3 years ago

No worries, I can definitely fix that import statement.

From what I understand, ITEM was a matplotlib related thing so iterating through the items caused issues with GDAL.

I still used it because it was where I could find the plot_type parameter outside of a loop that iterates through them.

I could add a conditional to check that ITEM1 even exists in the first place.

On Wed, Sep 22, 2021 at 3:10 PM Randall J. LeVeque @.***> wrote:

Reverts #282 https://github.com/clawpack/visclaw/pull/282

@AsianHam https://github.com/AsianHam: Sorry, I merged #282 https://github.com/clawpack/visclaw/pull/282 without properly testing it and it has at least a couple problems that render it inoperable on some standard examples.

In frametools.py:

import geoCon as gc should be from clawpack.visclaw import geoCon as gc

Line 183-184:

        if plotaxes.plotitem_dict['ITEM1'].plot_type == '2d_gtiff':
            gc.render(framesolns[0], frameno)

This is hardwired with the assumption that plotitem_dict has a key ITEM1. But if the user calls plotaxes.new_plotitem with the name argument then the default ITEM1 is not used.

Moreover, I don't understand why this call is made in the loop on plotfigure._axesnames rather than later in the embedded loop on plotaxes._itemnames that starts at line 320. Why is this done only for the first item?

But it's not clear to me what the use case is for this, so perhaps the original PR can be fixed up and a new PR issued with more explanation?

Thanks!

You can view, comment on, or merge this pull request online at:

https://github.com/clawpack/visclaw/pull/283 Commit Summary

File Changes

Patch Links:

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/clawpack/visclaw/pull/283, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGBXSXQ5TJI3QLQSVRPWVADUDIS37ANCNFSM5ESAXLQQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

rjleveque commented 3 years ago

ClawPlotAxes.new_plotitem is defined in visclaw.data and is where ITEMn is specified if name is not provided, with n incremented for each unnamed item.

In general a user may specify many plot items on a single plot axes, and the items might have different plot_type, e.g. perhaps one item with plot_type='2d_pcolor' and another with plot_type='2d_contour' to put contour lines on top of a pcolor plot, so it doesn't make sense to determine the plot_type outside the loop over items.

AsianHam commented 3 years ago

I'll take another look and re-PR

On Wed, Sep 22, 2021 at 3:42 PM Randall J. LeVeque @.***> wrote:

ClawPlotAxes.new_plotitem is defined in visclaw.data and is where ITEMn is specified if name is not provided, with n incremented for each unnamed item.

In general a user may specify many plot items on a single plot axes, and the items might have different plot_type, e.g. perhaps one item with plot_type='2d_pcolor' and another with plot_type='2d_contour' to put contour lines on top of a pcolor plot, so it doesn't make sense to determine the plot_type outside the loop over items.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/clawpack/visclaw/pull/283#issuecomment-925271394, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGBXSXSZD2TN6IY77TTI2OLUDIWTFANCNFSM5ESAXLQQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.