Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File names, import issues #172

Open
adamgayoso opened this issue Feb 4, 2022 · 0 comments
Open

File names, import issues #172

adamgayoso opened this issue Feb 4, 2022 · 0 comments

Comments

@adamgayoso
Copy link
Member

adamgayoso commented Feb 4, 2022

  1. File (module) names should be all lowercase see: https://www.python.org/dev/peps/pep-0008/#package-and-module-names -- and this is causing an issue with autosummary for nice class pages in the docs
  2. For an import like this
    from cassiopeia.data import utilities
    utilities should be a subpackage in that directory.
  3. I would avoid cross package imports like this
    from cassiopeia.solver import solver_utilities
    as I believe some parts of solver depend on the tree
  4. For an import like this
    from cassiopeia.solver import (
    DistanceSolver,
    dissimilarity_functions,
    solver_utilities,
    )
    I feel it gets kind of weird because solver_utilities is a file in solver but the other two are imported into that namespace via the init file.
  5. Imports like the first and third here
    from cassiopeia.data.CassiopeiaTree import CassiopeiaTree
    from cassiopeia.mixins import TreeSimulatorError
    from cassiopeia.simulator.TreeSimulator import TreeSimulator
    should not have the file name in them, as these are imported via the init of that directory
  6. I would rename the github repo to cassiopeia to remove the capital C so that the name reflects the python package name. All old urls will work still.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant