search

BCDA-APS / mdaviz

Data visualization for mda
https://bcda-aps.github.io/mdaviz/
Other
3 stars 0 forks source link

vendor the mdalib code #119

Closed prjemian closed 1 month ago

prjemian commented 2 months ago
prjemian commented 2 months ago

Is f_xdrlib.py necessary? It is not referenced but it will be necessary when xdrlib is dropped from the standard Python library in Python 3.13.

prjemian commented 2 months ago
prjemian commented 2 months ago

@rodolakis This is ready for review.

@timmmooney: Your comments are welcome.

timmmooney commented 2 months ago

I don't understand the purpose of uncommenting the f_xdrlib stuff. f_xdrlib still relies on xdrlib, which I understand to be the problem. However, I don't object to anyone trying to improve this code. I'm glad for it.

Tim Mooney @.***) (630)252-5417 Beamline Controls Group (www.aps.anl.gov) Advanced Photon Source, Argonne National Lab

From: Pete R Jemian @.> Sent: Wednesday, April 24, 2024 10:11 AM To: BCDA-APS/mdaviz @.> Cc: Mooney, Tim M. @.>; Mention @.> Subject: Re: [BCDA-APS/mdaviz] vendor the mdalib code (PR #119)

@rodolakis This is ready for review. @timmmooney: Your comments are welcome. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned. Message ID: BCDA-APS/mdaviz/pull/119/c2075183021@ github. com ZjQcmQRYFpfptBannerStart This Message Is From an External Sender This message came from outside your organization.

ZjQcmQRYFpfptBannerEnd

@rodolakishttps://urldefense.us/v3/__https://github.com/rodolakis__;!!G_uCfscf7eWS!ab6eWbbmnbnKJoWOVH0O-RZ8H8Fmex19IVCZfU75_rY67MX9KhmeRDnungCelTc5Qan0u-U-pp4swxwrYwctqBFK6A$ This is ready for review.

@timmmooneyhttps://urldefense.us/v3/__https://github.com/timmmooney__;!!G_uCfscf7eWS!ab6eWbbmnbnKJoWOVH0O-RZ8H8Fmex19IVCZfU75_rY67MX9KhmeRDnungCelTc5Qan0u-U-pp4swxwrYwcAw-Eg0w$: Your comments are welcome.

— Reply to this email directly, view it on GitHubhttps://urldefense.us/v3/__https://github.com/BCDA-APS/mdaviz/pull/119*issuecomment-2075183021__;Iw!!G_uCfscf7eWS!ab6eWbbmnbnKJoWOVH0O-RZ8H8Fmex19IVCZfU75_rY67MX9KhmeRDnungCelTc5Qan0u-U-pp4swxwrYwewbEYs4g$, or unsubscribehttps://urldefense.us/v3/__https://github.com/notifications/unsubscribe-auth/ABT6PF2AI2JVTHZUXVO6SK3Y67DS7AVCNFSM6AAAAABGW7BUKWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZVGE4DGMBSGE__;!!G_uCfscf7eWS!ab6eWbbmnbnKJoWOVH0O-RZ8H8Fmex19IVCZfU75_rY67MX9KhmeRDnungCelTc5Qan0u-U-pp4swxwrYwef-pZveg$. You are receiving this because you were mentioned.Message ID: @.***>

prjemian commented 2 months ago

@timmmooney -- Thanks for the reply.

https://github.com/BCDA-APS/mdaviz/blob/15847e8af7f8453e86a3a0ae713b12eb84440f6b/mdaviz/synApps_mdalib/f_xdrlib.py#L27

This is the only reference to xdrlib in f_xdrlib.py that I can find. (And, it's in a comment.) So, I assumed f_xdrlib had replaced all necessary code provided by the xdrlib package. If that is not the case, then we can reference an open-source alternative.

prjemian commented 2 months ago

I swapped the try..except block to prefer xdrlib. Starting with Py3.13 release, the code will use local f_xdrlib unless xdrlib is provided by a thrid-party package.

https://github.com/BCDA-APS/mdaviz/blob/15847e8af7f8453e86a3a0ae713b12eb84440f6b/mdaviz/synApps_mdalib/mda.py#L18-L23

timmmooney commented 2 months ago

My bad. I forgot how it worked. Your solution seems the best way to go.

Tim Mooney @.***) (630)252-5417 Beamline Controls Group (www.aps.anl.gov) Advanced Photon Source, Argonne National Lab

From: Pete R Jemian @.> Sent: Wednesday, April 24, 2024 11:18 AM To: BCDA-APS/mdaviz @.> Cc: Mooney, Tim M. @.>; Mention @.> Subject: Re: [BCDA-APS/mdaviz] vendor the mdalib code (PR #119)

@timmmooney -- Thanks for the reply. https: //github. com/BCDA-APS/mdaviz/blob/15847e8af7f8453e86a3a0ae713b12eb84440f6b/mdaviz/synApps_mdalib/f_xdrlib. py#L27 This is the only reference to xdrlib in f_xdrlib. py that I can find. (And, xdrlib is ZjQcmQRYFpfptBannerStart This Message Is From an External Sender This message came from outside your organization.

ZjQcmQRYFpfptBannerEnd

@timmmooneyhttps://urldefense.us/v3/__https://github.com/timmmooney__;!!G_uCfscf7eWS!c5MYMX9EQlXMdLhmM7Ge3mQDYNvs9OJFhpwi3Qe5bZ6H2vE7Cr91GEdAm3hSxUEik_WNdQHigfhp0eMOV15f5nWNGw$ -- Thanks for the reply.

https://github.com/BCDA-APS/mdaviz/blob/15847e8af7f8453e86a3a0ae713b12eb84440f6b/mdaviz/synApps_mdalib/f_xdrlib.py#L27https://urldefense.us/v3/__https://github.com/BCDA-APS/mdaviz/blob/15847e8af7f8453e86a3a0ae713b12eb84440f6b/mdaviz/synApps_mdalib/f_xdrlib.py*L27__;Iw!!G_uCfscf7eWS!c5MYMX9EQlXMdLhmM7Ge3mQDYNvs9OJFhpwi3Qe5bZ6H2vE7Cr91GEdAm3hSxUEik_WNdQHigfhp0eMOV14V1nxvTw$

This is the only reference to xdrlib in f_xdrlib.py that I can find. (And, xdrlib is not imported at this point.) So, I assumed f_xdrlib had replaced all necessary code provided by the xdrlib package. If that is not the case, then we can reference an open-source alternative.

— Reply to this email directly, view it on GitHubhttps://urldefense.us/v3/__https://github.com/BCDA-APS/mdaviz/pull/119*issuecomment-2075349599__;Iw!!G_uCfscf7eWS!c5MYMX9EQlXMdLhmM7Ge3mQDYNvs9OJFhpwi3Qe5bZ6H2vE7Cr91GEdAm3hSxUEik_WNdQHigfhp0eMOV17gOADtEA$, or unsubscribehttps://urldefense.us/v3/__https://github.com/notifications/unsubscribe-auth/ABT6PFYWF3IL7YMXO3HPTUDY67LNRAVCNFSM6AAAAABGW7BUKWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZVGM2DSNJZHE__;!!G_uCfscf7eWS!c5MYMX9EQlXMdLhmM7Ge3mQDYNvs9OJFhpwi3Qe5bZ6H2vE7Cr91GEdAm3hSxUEik_WNdQHigfhp0eMOV16_xk90pg$. You are receiving this because you were mentioned.Message ID: @.***>