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.

Sign up for GitHub

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

Jump to bottom

Refactor StrainCollection class #135

Merged
merged 22 commits into from
Apr 5, 2023
Merged

Refactor StrainCollection class #135

merged 22 commits into from
Apr 5, 2023

Conversation

CunliangGeng
Copy link
Member

Please first review #131 then this one.

Cleaned up the StrainCollection class and updated its unit tests.

@CunliangGeng
Update .prospector.yml for pylint
5b24aa5
@CunliangGeng
update type hints from Path to PathLike for utils
f5b1548
@CunliangGeng
remove unused method Strain.has_alias
6b12842
@CunliangGeng
change Strain.aliases from attribute to property
f657e2d 
Using property will force user to use method `add_alias` but not directly add alias to the set of aliases.
@CunliangGeng
add docstring to Strain class
e2032f6
@CunliangGeng
adjust the orders of methods in Strain class
001967d
@CunliangGeng
update strain unit tests
24fe7d9
@CunliangGeng
adjust method orders of StrainCollection class
0c8c962
@CunliangGeng
update docstrings and type hints in StrainCollection
a581d01 
add type hints
@CunliangGeng
update attribute names
4dea67e 
self._lookup -> self._strain_dict_id
self._lookup_indices -> self._strain_dict_index
@CunliangGeng
update __contains__ in StrainCollection
0ec47de 
update `__contains__` method
@CunliangGeng
refactor __eq__ method
652d18b
@CunliangGeng
refactor add method
40256d4
@CunliangGeng
refactor filter method
461db79
@CunliangGeng
update tests for remove and filter
5884f57
@CunliangGeng
update lookup method and its tests
de08d95
@CunliangGeng
add tests for magic methods
db46262
@CunliangGeng
update tests
4666069
@CunliangGeng
refactor add_from_file method
ae579aa
@CunliangGeng
update save_to_file method
568a560
@CunliangGeng
update generate_strain_mappings method
4e86b86
gcroci2
gcroci2 approved these changes Apr 5, 2023
View reviewed changes
Copy link
Contributor

@gcroci2 gcroci2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire logic is still a bit fuzzy for me, but the syntax, the type hints and the tests look good

@CunliangGeng
Copy link
Member Author

The entire logic is still a bit fuzzy for me, but the syntax, the type hints and the tests look good

The design of this class is not good and there will be further refactoring on it (issue #134).

@CunliangGeng CunliangGeng merged commit 5e58957 into dev Apr 5, 2023
@CunliangGeng CunliangGeng deleted the refactor_StrainCollection branch April 5, 2023 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gcroci2 gcroci2 gcroci2 approved these changes

@hechth hechth Awaiting requested review from hechth

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CunliangGeng @gcroci2