search

toshas / torch-fidelity

High-fidelity performance metrics for generative models in PyTorch
Other
1.01k stars 66 forks source link

Converts imports from absolute to relative #11

Closed voletiv closed 3 years ago

voletiv commented 3 years ago

Converts absolute path "from torch_fidelity. import ##" to relative "from . import ##". Also updates .gitignore with python related ignores.

toshas commented 3 years ago

Hi Vikram, my apologies for the delay, and thanks for the PR! As per our last conversation in https://github.com/toshas/torch-fidelity/pull/3#issuecomment-633095114, I have uploaded the CONTRIBUTING guidelines to indicate that a discussion in a GH issue should precede any PR. We can proceed with discussing this PR here this time. Also, I see that some of your PRs modify the LICENSE file. Unfortunately, I cannot accept these changes; however, should any of your commits be merged/cherry-picked into the main branch, your name will appear in the next version release BibTeX block, as generated by Zenodo. Thus, the credit assignment will be solved. I prefer to minimize the burden of API changes and unnecessary refactoring; there are better things to do with this package, such as adding new metrics and such -- if you have the bandwidth for that, we can discuss in a separate issue or email.

To the point of changing absolute imports to relative, could you please justify the need for this change? I checked a few prominent projects (links below), and I do not see a definitive pattern but rather a mixture of relative and absolute imports. PyTorch and sklearn use a mixture of abs and rel in many files: https://github.com/pytorch/pytorch/blob/master/torch/nn/modules/linear.py https://github.com/scikit-learn/scikit-learn/blob/main/sklearn/gaussian_process/kernels.py pytorch_lightning uses absolute imports only: https://github.com/PyTorchLightning/pytorch-lightning/blob/master/pytorch_lightning/loggers/tensorboard.py