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

fix LCA_Database __repr__ so that None is not displayed #1193

Open
ctb opened this issue Sep 8, 2020 · 9 comments
Open

fix LCA_Database __repr__ so that None is not displayed #1193

ctb opened this issue Sep 8, 2020 · 9 comments

Comments

@ctb
Copy link
Contributor

ctb commented Sep 8, 2020

reported by @bluegenes -- right now the __repr__ relies on self.filename, which is sometimes set to None.

The fix is to check if self.filename is None, and if so, just display LCA_Database(). See file sourmash/lca/lca_db.py.

@ctb
Copy link
Contributor Author

ctb commented Sep 8, 2020

More details for people interested in tackling this --

around line 163 of sourmash/lca/lca_db.py, the __repr__ function, currently:

def __repr__(self):
        return "LCA_Database('{}')".format(self.filename)

should return "LCA_Database()" when self.filename is None.

@nmb85
Copy link

nmb85 commented Sep 10, 2020

@bluegenes @ctb Is this for the core team? Or is this something that anyone - even a first-time contributor to open source software - is suited to fix? If this is for anyone, then I would sheepishly like to offer to try to fix it. I have no experience contributing to OSS, and I am by no means a Python expert, and I really, really don't want to be an imposition by offering to do something that would create more work for the core team than it would save. I would have to learn the github procedures somehow - is there a guide that this project adheres to/prefers? - and then fix the code and submit it to someone for review, etc, etc. But I would love the chance to just see what it's like at least once.

@ctb
Copy link
Contributor Author

ctb commented Sep 10, 2020

Hi Nathan,

absolutely, go for it!

The rough workflow is GitHub flow -

  1. fork sourmash to your own github account using the fork button (upper right)
  2. clone from that to your computer (wherever you work on code)
  3. set up a developer environment
  4. run the tests
  5. create a branch in your working location
  6. begin making changes
  7. commit changes, etc. etc., until you've gotten somewhere
  8. push changes back to https://github.com/nmb85/sourmash under new branch name
  9. set up pull request from there to dib-lab/sourmash
  10. we'll review your pull request!

so, lots of moving parts, now that I write it out, but what is now a fairly standard workflow for github projects, so it's worth learning if you haven't done it before!

@ctb
Copy link
Contributor Author

ctb commented Sep 10, 2020

(happy to take questions here or elsewhere as you have them)

@nmb85
Copy link

nmb85 commented Sep 10, 2020

Thank you for the opportunity! I will spend the next few days learning the workflow and trying to follow procedure to make this tiny little contribution. Then, if it's okay with the team, I'll search for other small "good first issues" to hoover up and try to clean up little bits and bobs of code here and there so that I can grok the process.

@nmb85
Copy link

nmb85 commented Sep 11, 2020

  1. set up a developer environment

My system is Debian Testing with all sorts of rubbish installed, so I'm wondering if messy environments like that often contribute artifacts to these sorts of projects or if it's something I can mostly ignore. Are there any special directions for this; beyond isolating my branch repository installation from another sourmash installation (easy enough), is it necessary to set up Docker and track every little detail of my environment or do you think it's good enough to just use a dedicated Conda Python3 environment/virtualenv? Maybe it's enough just to be able to track and communicate my env? Other than that, I'll get down to it.

@ctb
Copy link
Contributor Author

ctb commented Sep 11, 2020

oh, sorry, forgot the link it seems! https://sourmash.readthedocs.io/en/latest/developer.html

nothing special is needed! I use conda myself. Our automated tests run in clean environments, note.

the conda install instructions here,

https://sourmash.readthedocs.io/en/latest/release.html

should work.

@nmb85
Copy link

nmb85 commented Sep 12, 2020

Thanks for the detailed link!

nothing special is needed! I use conda myself. Our automated tests run in clean environments, note.

Noted

@ctb
Copy link
Contributor Author

ctb commented Mar 12, 2022

hah, I thought for sure we'd fixed this in #1837 or something, but no!

>>> db = LCA_Database(31, 1000)
>>> db
LCA_Database('None')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants