root icon indicating copy to clipboard operation
root copied to clipboard

Published 20 hours ago • verified publisher icon root-project

[GSoC] Hist and TGraph Pythonizations.

Open harshal0815 opened this issue 3 years ago • 4 comments

This Pull request:

Changes or fixes:

Checklist:

  • [ ] tested changes locally
  • [ ] updated the docs (if necessary)

This PR fixes #

harshal0815 avatar Sep 13 '22 14:09 harshal0815

Can one of the admins verify this patch?

phsft-bot avatar Sep 13 '22 14:09 phsft-bot

@etejedor Sir, Can you kindly review this PR. Also, there seems to be an issue with the TGraph constructor pythoniztion and the GetAsNumpy methods. Please post any changes or additions I can make. I will update the PR soon accordingly based on your inputs and suggestions. Thank you.

Here's a link to the github gist for your reference.

harshal0815 avatar Sep 22 '22 10:09 harshal0815

Hello @Harshalzzzzzzz

Also, there seems to be an issue with the TGraph constructor pythoniztion and the GetAsNumpy methods

What is exactly the problem?

etejedor avatar Sep 22 '22 11:09 etejedor

TGraph Constructor returns a null value @etejedor

harshal0815 avatar Sep 22 '22 11:09 harshal0815

TGraph Constructor returns a null value

Please see my comments inline in the code about this.

etejedor avatar Sep 22 '22 12:09 etejedor

@etejedor Can you also please review the GetAsNumpy method, so that I can resolve the errors

harshal0815 avatar Sep 23 '22 06:09 harshal0815

@etejedor Can you also please review the GetAsNumpy method, so that I can resolve the errors

What kind of errors do you get?

etejedor avatar Sep 23 '22 07:09 etejedor

What kind of errors do you get?

@etejedor Thank you sir, both errors are resolved, Kindly review the PR if possible. Regards.

harshal0815 avatar Sep 27 '22 11:09 harshal0815

@lmoneta @sanjibansg @omazapa PR updated. Kindly review, I will update the fixes/changes, if any, asap.

Thanks.

harshal0815 avatar Oct 03 '22 08:10 harshal0815

@guitargeek We have already FillN to fill a numpy array in an histogram. I think is good to have a function creating and filling histogram directly from Numpy passing the array and the other c'tor arguments (number of bins, min and max), or just number of bins and compute min and max automatically.

lmoneta avatar Oct 03 '22 13:10 lmoneta

Okay! Having FromNumpy then makes sense, it just needs some proper docstring

guitargeek avatar Oct 03 '22 13:10 guitargeek

This pull request introduces 4 alerts when merging cb774a42274505bebdb77f6921408802253aff9f into 2441d1359ddd4d3a644681f3208f9e3b0282464d - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 2 for Module is imported with 'import' and 'import from'

lgtm-com[bot] avatar Oct 09 '22 16:10 lgtm-com[bot]

This pull request introduces 4 alerts when merging 3285d602fd1b130aa17096b5b066a95368f05005 into b7a3cb5ada6c71119f068c488fec0161d58d2dd7 - view on LGTM.com

new alerts:

  • 3 for Unused import
  • 1 for Wrong number of arguments in a call

lgtm-com[bot] avatar Oct 14 '22 10:10 lgtm-com[bot]

This pull request introduces 4 alerts when merging 538d1cf59476f91dbb49d350875b86b0a6c7eb51 into 97802b9066ca98c17527949b76b9f070ce676738 - view on LGTM.com

new alerts:

  • 3 for Unused import
  • 1 for Wrong number of arguments in a call

lgtm-com[bot] avatar Oct 17 '22 08:10 lgtm-com[bot]

This pull request introduces 1 alert when merging b5f4c785eafff9b94765049f160dc5989ca6b126 into e7c7170af9cddaa6482763db49a20ef55609327c - view on LGTM.com

new alerts:

  • 1 for Wrong number of arguments in a call

lgtm-com[bot] avatar Oct 20 '22 13:10 lgtm-com[bot]