MDAnalysis / panedr

Reads Gromacs EDR file to populate a pandas dataframe
GNU Lesser General Public License v2.1
31 stars 7 forks source link

Fix license / authors / readme / changelog packaging issues #63

Closed IAlibay closed 1 year ago

IAlibay commented 1 year ago

I tried my best to avoid having to do this, but after far far too long delving into the setuptools code it seems that this might be the only way forward (indeed the idea of doing multiple packages in the way we're doing is something that's generally considered not standard practice).

To ensure that things are properly packaged I'm adding symlinks here for all the files which are shared between the pyedr and panedr package directories.

codecov[bot] commented 1 year ago

Codecov Report

Base: 82.97% // Head: 82.97% // No change to project coverage :thumbsup:

Coverage data is based on head (c0fc87d) compared to base (1094467). Patch has no changes to coverable lines.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #63 +/- ## ======================================= Coverage 82.97% 82.97% ======================================= Files 3 3 Lines 276 276 ======================================= Hits 229 229 Misses 47 47 ``` Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MDAnalysis). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MDAnalysis)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

IAlibay commented 1 year ago

@BFedder @orbeckst @jbarnoud @hmacdope - can I get a general yes/no vote on these symmlinked files? I know it's a bit contentious but I'm out of ideas on how to deal with this (I ended up doing what folks like rapids do when they have multiple packages per repo).

IAlibay commented 1 year ago

I assume that this will also work on Windows?

That's a tough one, so:

1) Will the packages work on Windows - Yes, on packaging the files are copied not symlinked so the tarballs have the right files 2) Will windows folks be able to install from github source - I haven't tested to be honest, I suspect with pip installing via local wheel creation that might be a no. I'll see if I can give it a go in a few.

jbarnoud commented 1 year ago

I am all for symlinks. May question is also about Windows : will there links survive a pull and push from a Windows machine that is not configured to have symlinks? Mine is so I cannot test.

BFedder commented 1 year ago

Will windows folks be able to install from github source - I haven't tested to be honest, I suspect with pip installing via local wheel creation that might be a no. I'll see if I can give it a go in a few.

I just tried installing panedr and pyedr from source on my windows machine. For both the master branch and the fix-license-packaging branch, that only worked for me when I used pip install -e . instead of pip install .. When installing non-editable, pbr complained as such:

 Complete output (16 lines):
    ERROR:root:Error parsing
    Traceback (most recent call last):
      File "c:\users\bjarne\new\lib\site-packages\pbr\core.py", line 111, in pbr
        attrs = util.cfg_to_args(path, dist.script_args)
      File "c:\users\bjarne\new\lib\site-packages\pbr\util.py", line 272, in cfg_to_args
        pbr.hooks.setup_hook(config)
      File "c:\users\bjarne\new\lib\site-packages\pbr\hooks\__init__.py", line 25, in setup_hook
        metadata_config.run()
      File "c:\users\bjarne\new\lib\site-packages\pbr\hooks\base.py", line 27, in run
        self.hook()
      File "c:\users\bjarne\new\lib\site-packages\pbr\hooks\metadata.py", line 25, in hook
        self.config['version'] = packaging.get_version(
      File "c:\users\bjarne\new\lib\site-packages\pbr\packaging.py", line 874, in get_version
        raise Exception("Versioning for this project requires either an sdist"
    Exception: Versioning for this project requires either an sdist tarball, or access to an upstream git repository. It's also possible that there is a mismatch between the package name in setup.cfg and the argument given to pbr.version.VersionInfo. Project name pyedr was given, but was not able to be found.
    error in pyedr setup command: Error parsing C:\Users\Bjarne\AppData\Local\Temp\pip-req-build-kg5ore8w\setup.cfg: Exception: Versioning for this project requires either an sdist tarball, or access to an upstream git repository. It's also possible that there is a mismatch between the package name in setup.cfg and the argument given to pbr.version.VersionInfo. Project name pyedr was given, but was not able to be found.
    ----------------------------------------
ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.

I'm not sure if this is a problem caused by my setup or by the package, though, as I don't have much experience dealing with these things on windows. Not sure how critical this would be either way, and in general, I like the symlinks.

BFedder commented 1 year ago

I added an empty file and pushed here from my Windows setup to test if the symlinks behave okay

BFedder commented 1 year ago

I'm running Windows 10 Pro version 21H2, OS build 19044.2251

IAlibay commented 1 year ago

I added an empty file and pushed here from my Windows setup to test if the symlinks behave okay

Thanks @BFedder ! Looks like they survived the roundtrip.

IAlibay commented 1 year ago

Will windows folks be able to install from github source - I haven't tested to be honest, I suspect with pip installing via local wheel creation that might be a no. I'll see if I can give it a go in a few.

I just tried installing panedr and pyedr from source on my windows machine. For both the master branch and the fix-license-packaging branch, that only worked for me when I used pip install -e . instead of pip install .. When installing non-editable, pbr complained as such:

 Complete output (16 lines):
    ERROR:root:Error parsing
    Traceback (most recent call last):
      File "c:\users\bjarne\new\lib\site-packages\pbr\core.py", line 111, in pbr
        attrs = util.cfg_to_args(path, dist.script_args)
      File "c:\users\bjarne\new\lib\site-packages\pbr\util.py", line 272, in cfg_to_args
        pbr.hooks.setup_hook(config)
      File "c:\users\bjarne\new\lib\site-packages\pbr\hooks\__init__.py", line 25, in setup_hook
        metadata_config.run()
      File "c:\users\bjarne\new\lib\site-packages\pbr\hooks\base.py", line 27, in run
        self.hook()
      File "c:\users\bjarne\new\lib\site-packages\pbr\hooks\metadata.py", line 25, in hook
        self.config['version'] = packaging.get_version(
      File "c:\users\bjarne\new\lib\site-packages\pbr\packaging.py", line 874, in get_version
        raise Exception("Versioning for this project requires either an sdist"
    Exception: Versioning for this project requires either an sdist tarball, or access to an upstream git repository. It's also possible that there is a mismatch between the package name in setup.cfg and the argument given to pbr.version.VersionInfo. Project name pyedr was given, but was not able to be found.
    error in pyedr setup command: Error parsing C:\Users\Bjarne\AppData\Local\Temp\pip-req-build-kg5ore8w\setup.cfg: Exception: Versioning for this project requires either an sdist tarball, or access to an upstream git repository. It's also possible that there is a mismatch between the package name in setup.cfg and the argument given to pbr.version.VersionInfo. Project name pyedr was given, but was not able to be found.
    ----------------------------------------
ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.

I'm not sure if this is a problem caused by my setup or by the package, though, as I don't have much experience dealing with these things on windows. Not sure how critical this would be either way, and in general, I like the symlinks.

Regarding this, I'm not sure at the moment, but it's definitely a pbr issue. I think it's because it's not properly working with the way pip install works on wheel generation.

IAlibay commented 1 year ago

I'd like to get this pushed upstream ASAP. Since @BFedder confirmed that the symlinks still work after a windows push, I'm going to go ahead with this and we can fix the weird pbr issues at a later date.