edit intro.rst#90
Conversation
havok2063
left a comment
There was a problem hiding this comment.
I don't know if the variable names are particularly confusing. If a user is learning Python, they should be learning the difference between a class name/object and variable name/object. I'm hesitant to put the release in the variable name. I've suggested some alternative variable names to use through the docs.
| As noted above, most users will only need to use the `.Access` class. | ||
| Use the `.HttpAccess`, `.RsyncAccess`, and `.CurlAccess` classes only if you have some specific reason to use them instead of the `.Access` class. | ||
| Note that the syntax for `.HttpAccess` is different from the syntax for `.Access`, `.RsyncAccess`, and `.CurlAccess`. | ||
|
|
There was a problem hiding this comment.
Access does not select HttpAccess. It only toggles between RsyncAccess and CurlAccess. HttpAccess is a different download mechanism.
| adding paths to ensure successful downloading of data. | ||
| adding paths to ensure successful downloading of data. | ||
|
|
||
| Using the `.HttpAccess` class. |
There was a problem hiding this comment.
It looks like this line has been removed. Can you add it back in down before the :: at line 254?
| http_access_dr17 = HttpAccess(release='DR17', verbose=True) | ||
|
|
||
| # set to use remote | ||
| http_access.remote() | ||
| http_access_dr17.remote() | ||
|
|
||
| # get the file | ||
| http_access.get('mangacube', drpver='v3_1_1', plate='8485', ifu='1901', wave='LOG') | ||
| http_access_dr17.get('mangacube', drpver='v3_1_1', plate='8485', ifu='1901', wave='LOG') | ||
|
|
There was a problem hiding this comment.
I think http_access is fine, but if you want to rename, maybe http or myhttp.
| path_dr17 = Path(release='dr17') | ||
|
|
||
| # generate a file system path | ||
| path.full('mangacube', drpver='v3_1_1', plate='8485', ifu='1901', wave='LOG') | ||
| path_dr17.full('mangacube', drpver='v3_1_1', plate='8485', ifu='1901', wave='LOG') | ||
| '/Users/Brian/Work/sdss/sas/dr17/manga/spectro/redux/v3_1_1/8485/stack/manga-8485-1901-LOGCUBE.fits.gz' |
There was a problem hiding this comment.
I'm not a huge fan of putting the data release in the variable name. It's maybe a little more confusing, and if we change the release example in the future, it's more docs to change.
If you want to rename the variable, I might suggest mypath or pathobj.
| # import the rsync class | ||
| from sdss_access import RsyncAccess | ||
| rsync = RsyncAccess(release='DR17') | ||
| rsync_dr17 = RsyncAccess(release='DR17') |
There was a problem hiding this comment.
I think rsync is distinct enough from RsyncAccess to cause confusion, but if you want to rename maybe myrsync or rsyncobj?
| # import the curl class | ||
| from sdss_access import CurlAccess | ||
| curl = CurlAccess(release='DR17') | ||
| curl_dr17 = CurlAccess(release='DR17') |
There was a problem hiding this comment.
same thing with curl and CurlAccess. Maybe mycurl or curlobj.
|
Hi Brian,
I have made the changes.
I have replaced variable names like 'path_dr17' by 'mypath'.
Thanks!
Pramod
________________________________________
From: Brian Cherinka ***@***.***>
Sent: Tuesday, May 5, 2026 8:56 AM
To: sdss/sdss_access
Cc: Pramod Gupta; Author
Subject: Re: [sdss/sdss_access] edit intro.rst (PR #90)
@havok2063 requested changes on this pull request.
I don't know if the variable names are particularly confusing. If a user is learning Python, they should be learning the difference between a class name/object and variable name/object. I'm hesitant to put the release in the variable name. I've suggested some alternative variable names to use through the docs.
________________________________
In docs/sphinx/intro.rst<https://urldefense.com/v3/__https://github.com/sdss/sdss_access/pull/90*discussion_r3189762910__;Iw!!K-Hz7m0Vt54!k2eCuJOY5aeF6nbkATjdKR-qR4JOhNZSz3KdxpEF54nbIxg92u4rny7djkUJb0rz6ciNhOBkHbJbwralIBlLJfo$>:
+As noted above, most users will only need to use the `.Access` class.
+Use the `.HttpAccess`, `.RsyncAccess`, and `.CurlAccess` classes only if you have some specific reason to use them instead of the `.Access` class.
+Note that the syntax for `.HttpAccess` is different from the syntax for `.Access`, `.RsyncAccess`, and `.CurlAccess`.
Access does not select HttpAccess. It only toggles between RsyncAccess and CurlAccess. HttpAccess is a different download mechanism.
________________________________
In docs/sphinx/intro.rst<https://urldefense.com/v3/__https://github.com/sdss/sdss_access/pull/90*discussion_r3189774249__;Iw!!K-Hz7m0Vt54!k2eCuJOY5aeF6nbkATjdKR-qR4JOhNZSz3KdxpEF54nbIxg92u4rny7djkUJb0rz6ciNhOBkHbJbwral42vHMjQ$>:
-Using the `.HttpAccess` class.
It looks like this line has been removed. Can you add it back in down before the :: at line 254?
________________________________
In docs/sphinx/intro.rst<https://urldefense.com/v3/__https://github.com/sdss/sdss_access/pull/90*discussion_r3189784014__;Iw!!K-Hz7m0Vt54!k2eCuJOY5aeF6nbkATjdKR-qR4JOhNZSz3KdxpEF54nbIxg92u4rny7djkUJb0rz6ciNhOBkHbJbwralrhNgXF0$>:
+ http_access_dr17 = HttpAccess(release='DR17', verbose=True)
# set to use remote
- http_access.remote()
+ http_access_dr17.remote()
# get the file
- http_access.get('mangacube', drpver='v3_1_1', plate='8485', ifu='1901', wave='LOG')
+ http_access_dr17.get('mangacube', drpver='v3_1_1', plate='8485', ifu='1901', wave='LOG')
I think http_access is fine, but if you want to rename, maybe http or myhttp.
________________________________
In docs/sphinx/intro.rst<https://urldefense.com/v3/__https://github.com/sdss/sdss_access/pull/90*discussion_r3189796747__;Iw!!K-Hz7m0Vt54!k2eCuJOY5aeF6nbkATjdKR-qR4JOhNZSz3KdxpEF54nbIxg92u4rny7djkUJb0rz6ciNhOBkHbJbwral2UdbRlY$>:
+ path_dr17 = Path(release='dr17')
# generate a file system path
- path.full('mangacube', drpver='v3_1_1', plate='8485', ifu='1901', wave='LOG')
+ path_dr17.full('mangacube', drpver='v3_1_1', plate='8485', ifu='1901', wave='LOG')
'/Users/Brian/Work/sdss/sas/dr17/manga/spectro/redux/v3_1_1/8485/stack/manga-8485-1901-LOGCUBE.fits.gz'
I'm not a huge fan of putting the data release in the variable name. It's maybe a little more confusing, and if we change the release example in the future, it's more docs to change.
If you want to rename the variable, I might suggest mypath or pathobj.
________________________________
In docs/sphinx/intro.rst<https://urldefense.com/v3/__https://github.com/sdss/sdss_access/pull/90*discussion_r3189803014__;Iw!!K-Hz7m0Vt54!k2eCuJOY5aeF6nbkATjdKR-qR4JOhNZSz3KdxpEF54nbIxg92u4rny7djkUJb0rz6ciNhOBkHbJbwral7Wfj9_Y$>:
@@ -238,52 +269,30 @@ file downloads across multiple continuous rsync download streams.
# import the rsync class
from sdss_access import RsyncAccess
- rsync = RsyncAccess(release='DR17')
+ rsync_dr17 = RsyncAccess(release='DR17')
I think rsync is distinct enough from RsyncAccess to cause confusion, but if you want to rename maybe myrsync or rsyncobj?
________________________________
In docs/sphinx/intro.rst<https://urldefense.com/v3/__https://github.com/sdss/sdss_access/pull/90*discussion_r3189807951__;Iw!!K-Hz7m0Vt54!k2eCuJOY5aeF6nbkATjdKR-qR4JOhNZSz3KdxpEF54nbIxg92u4rny7djkUJb0rz6ciNhOBkHbJbwralavHuTOI$>:
Using the `.CurlAccess` class. `.CurlAccess` behaves exactly the same way as `.RsyncAccess`. After importing and
instantiating a `.CurlAccess` object, all methods and behavior are the same as in the `.RsyncAccess` class.
::
# import the curl class
from sdss_access import CurlAccess
- curl = CurlAccess(release='DR17')
+ curl_dr17 = CurlAccess(release='DR17')
same thing with curl and CurlAccess. Maybe mycurl or curlobj.
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https://github.com/sdss/sdss_access/pull/90*pullrequestreview-4229582632__;Iw!!K-Hz7m0Vt54!k2eCuJOY5aeF6nbkATjdKR-qR4JOhNZSz3KdxpEF54nbIxg92u4rny7djkUJb0rz6ciNhOBkHbJbwralGbhFQ4k$>, or unsubscribe<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AOTAX45PAGMNXWS7L36KMMT4ZIFLVAVCNFSM6AAAAACYRSLCA2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DEMRZGU4DENRTGI__;!!K-Hz7m0Vt54!k2eCuJOY5aeF6nbkATjdKR-qR4JOhNZSz3KdxpEF54nbIxg92u4rny7djkUJb0rz6ciNhOBkHbJbwralYjUaaWY$>.
Triage notifications on the go with GitHub Mobile for iOS<https://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!K-Hz7m0Vt54!k2eCuJOY5aeF6nbkATjdKR-qR4JOhNZSz3KdxpEF54nbIxg92u4rny7djkUJb0rz6ciNhOBkHbJbwralyXQutOE$> or Android<https://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign*3Dnotification-email*26utm_medium*3Demail*26utm_source*3Dgithub__;JSUlJSU!!K-Hz7m0Vt54!k2eCuJOY5aeF6nbkATjdKR-qR4JOhNZSz3KdxpEF54nbIxg92u4rny7djkUJb0rz6ciNhOBkHbJbwralv-wNEmk$>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
No description provided.