Paths and parses

[dllahr]

@oena @levlitichev I think this solves the problem of allowing absolute imports and having to avoid parse.parse.

[oena]

@dllahr I feel like I'm missing something here. My understanding was that you were concerned about modifying sys.path--which if so is a larger issue than just involving the parsing modules. It seems to me that while your code changes seems like a reasonable way to preserve prior parse() behavior without modifying the path just for the parsing-related modules, the same sys.path behavior is persisting in all the other modules.

If you have a way to get around the sys.path modification for all the modules (which I thought was what we were discussing yesterday?) I think that change is great to make to the repo. But I'm not sure I follow how changing it just for the parse modules--while everything else keeps modifying the sys.path--is better than what is on the master branch currently (which also preserved parse() instead of parse.parse() for function calls).

[dllahr]

I just made the change for parse as an example to demonstrate the idea, but I believe we can now remove from sys.path everywhere, and can use standardized, absolute path imports. If we have any other module we want to behave as callable (I don't think we do) we could do a similar pattern.

On Wed, Jan 10, 2018 at 11:17 AM, Oana Enache notifications@github.com wrote:

@dllahr https://github.com/dllahr I feel like I'm missing something here. My understanding was that you were concerned about modifying sys.path--which if so is a larger issue than just involving the parsing modules. It seems to me that while your code changes seems like a reasonable way to preserve prior parse() behavior without modifying the path just for the parsing-related modules, the same sys.path behavior is persisting in all the other modules.

If you have a way to get around the sys.path modification for all the modules (which I thought was what we were discussing yesterday?) I think that change is great to make to the repo. But I'm not sure I follow how changing it just for the parse modules--while everything else keeps modifying the sys.path--is better than what is on the master branch currently (which also preserved parse() instead of parse.parse() for function calls).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cmap/cmapPy/pull/26#issuecomment-356652347, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfjIlnFXz2MBsSU4aDrGCJx1ZRGdrWYks5tJOKJgaJpZM4RYvDb .

--

617 714 7868 dlahr@broadinstitute.org

[levlitichev]

Dave, this feels pretty hack-y, don't you think? Based on our many conversations, my preference would be to allow parse as a relative import; it's unique because it's the only method, rather than module, we import. This way we don't have to 1) sneak anything into the init.py, 2) sneak anything into sys.path, or 3) go through these gymnastics here.

In the 3 commits above, I went through and (painstakingly) removed all of the sys.path.insert(...) calls everywhere, made the import of parse relative, and removed six (since we're punting on py3 compatibility).

Let me know what you think.

[dllahr]

To recap our in-person conversation - agree it is hacky, agree with your plan to fix

On Tue, Feb 13, 2018 at 12:31 PM, Lev Litichevskiy <notifications@github.com

wrote:

Dave, this feels pretty hack-y, don't you think? Based on our many conversations, my preference would be to allow parse as a relative import; it's unique because it's the only method, rather than module, we import. This way we don't have to 1) sneak anything into the init.py, 2) sneak anything into sys.path, or 3) go through these gymnastics here.

In the 3 commits above, I went through and (painstakingly) removed all of the sys.path.insert(...) calls everywhere, made the import of parse relative, and removed six (since we're punting on py3 compatibility).

Let me know what you think.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cmap/cmapPy/pull/26#issuecomment-365341884, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfjIgsdH1JXioooIc9qbqeK14tGQPZoks5tUcbmgaJpZM4RYvDb .

--

617 714 7868 dlahr@broadinstitute.org

[levlitichev]

Final comment before merging: we decided to make all import statements absolute and to use the import PACKAGE.MODULE.SUBMODULE> as <NAME> structure, even for parse.py.