Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Defined the test_clabel function in the test_datetime file#27490

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
Ali-Aboelela wants to merge6 commits intomatplotlib:mainfromAli-Aboelela:test-datetime-clabel
Closed

Defined the test_clabel function in the test_datetime file#27490

Ali-Aboelela wants to merge6 commits intomatplotlib:mainfromAli-Aboelela:test-datetime-clabel

Conversation

Ali-Aboelela
Copy link

@Ali-AboelelaAli-Aboelela commentedDec 10, 2023
edited
Loading

PR summary

This PR is one of the issues identified by the larger issue#26864. These changes add a test for the clabel function. I have tested passing datetimes as input labels to the contours and the axis.I could not think of a way nor do I think it to make sense to pass datetime objects as labels to either axis.

Edit: Attempt 3

This time I correctly generate the contour using datetime objects and directly pass it to the clabel function. Passing a contour that was generated where either X or Y values contain datetime objects functions correctly. I simply avoided doing any math related to the inputs to find Z. TheHovomoller diagram@rcomer mentioned helped me visualize how this kind of contour plot with dates would be used which helped me form an example.

Note: I have NOT been able to specify the locations of the labels using the manual kwarg using datetime objects, please see discussion/comments below

Note: I did not use the example where x-axis was used as a date (commented in the code) because it was quite messy, instead I've provided the plot generated when datetime objects are used to set the y-axis of a contour plot.

download

Edit: Attempt 2

  1. Passing datetime objects to the clabel function works as expected and labels the contours as seen in both the above and below imgs
  2. Trying to generate contours using datetime objects and thus have the axis be labeled naturally as you would with numbers fails (any kind of math you do with numpy will fail).
  3. You can, however use ax.set_ticks and the ax.set_xticklabels functions, passing datetime objects directly if you wish, to label the x-axis of a contour plot. I do not know however, if this is what the original issue was attempting to test. In any case, the following is the plot produced:

download1

Original PR summary (Attempt 1)

Here is the image generated by this code:

download

PR checklist

  • [N/A] "closes #0000" is in the body of the PR description tolink the related issue

  • new and changed code istested
    I have struggled to run pytest due to a circular import issue. Instead, the image of the plot was generated using a separate script.

  • [N/A]Plotting related features are demonstrated in anexample

  • [N/A]New Features andAPI Changes are noted with adirective and release note

  • [N/A] Documentation complies withgeneral anddocstring guidelines

@jklymak
Copy link
Member

I think the test here is that the contour x and/or y axes are datetimes, and that the manual placement of clabels can be specified with datetimes. I'm actually skeptical that this works, but if not, would be good to document it.

@Ali-Aboelela
Copy link
Author

Ali-Aboelela commentedDec 10, 2023
edited
Loading

How would I be able to test this? I've tried a few different things but only ran into errors.

Edit: Perhaps this PR should be modified to just delete the test? I don't think what#26864 is attempting to test makes sense for clabel.

Copy link

@github-actionsgithub-actionsbot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join uson gitter for real-time discussion.

For details on testing, writing docs, and our review process, please seethe developer guide

We strive to be a welcoming and open project. Please follow ourCode of Conduct.

@jklymak
Copy link
Member

jklymak commentedDec 10, 2023
edited
Loading

@AliAboelela123 I've not looked in detail, but it is possible to contour withx ory as a datetime vector.z should just be normal non-datetime data. First, it would be madly useful to double check thatclabel works at all for that case. Second,clabel takes amanual kwarg, and the x, y version of that kwarg would ideally take datetime as thex value. I'm not sure if it does or not.

https://matplotlib.org/stable/api/contour_api.html#matplotlib.contour.ContourLabeler.clabel

@rcomer
Copy link
Member

rcomer commentedDec 10, 2023
edited
Loading

One use-case for z datetimes is to plot amap of monsoon onset, though I agree that having datetimes on the x- or y-axis (e.g.Hovmöller diagrams) is more common. Perhaps it is worth testing both?

@rcomer
Copy link
Member

I've tried a few different things but only ran into errors.

If the errors show that we don't currently support this, then that is useful information and part of what the issue is looking to find out. So please post your findings 👍

@Ali-Aboelela
Copy link
Author

I've made some updates, I don't expect this to be merged soon but rather just trying to continue the conversation. I summarized my findings in an edit

@jklymak
Copy link
Member

@AliAboelela123 I still think you are misunderstanding. The x array should be datetimebefore sending to contour. Subsequently clabel should be called with the manual kwarg.

@jklymak
Copy link
Member

Perhaps it is worth testing both

The z data in contour is not run through unit machinery, so all we would be testing is that clabel takes an arbitrary string.

rcomer reacted with thumbs up emojircomer reacted with eyes emoji

…etime objects directly to contour when creating contour plot using clabel
@Ali-Aboelela
Copy link
Author

Ok I've tried to fix the PR by constructing the contour using datetime objects and passing that as input to the clabel function. I have not used the manual kwarg though. Do the current changes more accurately reflect what this test suite is attempting to test?

@jklymak
Copy link
Member

Subsequently clabel should be called with the manual kwarg.

I have not used the manual kwarg though. Do the current changes more accurately reflect what this test suite is attempting to test?

No? What are you not understanding about the "manual" kwarg?

@Ali-Aboelela
Copy link
Author

Ali-Aboelela commentedDec 10, 2023
edited
Loading

# Select specific x valuesselected_x_values = [x[0], x[6], x[12], x[18]]#Pair them with specific datesselected_dates = [dates[0], dates[3], dates[6], dates[9]]#Create pairs of (x, date)manual = [(x_val, date) for x_val, date in zip(selected_x_values, selected_dates)

I tried to pass the above ^^ "manual" param to the clabel function but I get the following error:

"TypeError: Cannot cast array data from dtype('O') to dtype('float64') according to the rule 'safe'"

This makes me think that the manual param expects the tuples to have float type i.e. coordinates in a plane rather than data type 'O' which I think means object and is referencing the datetime obj. I'm not sure if I'm doing something wrong or if this is the intended limitation of the clabel function which we are attempting to document.

@jklymak
Copy link
Member

I didn't test it myself, but if the manual clabel doesn't work then that would be a bug report issue. Probably we should process the unit info for the clabel manual inputs.

This PR could still test that labelling works when the x and y axis are datetimes. Somewhat more trivial, but worth checking.

@QuLogic
Copy link
Member

I've opened#27525 for themanual argument ofclabel not accepting units. This PR is possibly okay without testing that yet, but it currently has linting issues that need to be fixed.

Copy link
Member

@ksundenksunden left a comment

Choose a reason for hiding this comment

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

I don't think the code as it is written here is exercising the datetimes at all really, it is only using them as labels (passed throughstr, not processed through our unit system)

I think that themanual behavior described in#27525 would be a good thing to test here, but that doesn't yet work

As for datetimes on the Z axis, we simply don't have any native support for that at this time, and that would be a larger change.

@Ali-AboelelaAli-Aboelela closed this by deleting the head repositorySep 13, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@github-actionsgithub-actions[bot]github-actions[bot] left review comments

@ksundenksundenksunden requested changes

Assignees
No one assigned
Projects
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@Ali-Aboelela@jklymak@rcomer@QuLogic@ksunden@melissawm

[8]ページ先頭

©2009-2025 Movatter.jp