Skip to content
This repository has been archived by the owner on Jan 29, 2024. It is now read-only.

Add the TopicInfo data structure #542

Merged
merged 26 commits into from
Jan 19, 2022

Conversation

Stannislav
Copy link
Contributor

@Stannislav Stannislav commented Jan 10, 2022

Fixes #539 .

Description

This adds the TopicInfo data class that replaces the manual creation and copy-pasting of the structured dictionary that we have been using for this purpose so far.

How to test?

  • See new tests under tests/unit/database/test_topic_info.py
  • Adjusted tests for old code pass

Checklist

  • This PR refers to an issue from the issue tracker.
    (if it is not the case, please create an issue first).
  • Unit tests added.
    (if needed)
  • Documentation and whatsnew.rst updated.
    (if needed)
  • setup.py and requirements.txt updated with new dependencies.
    (if needed)
  • Type annotations added.
    (if a function is added or modified)
  • All CI tests pass.

@Stannislav Stannislav self-assigned this Jan 10, 2022
Copy link
Contributor

@EmilieDel EmilieDel left a comment

Choose a reason for hiding this comment

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

Really nice 🎉 It is definitely more clear and clean than copy-pasting the dictionary for every source, thanks for the changes!

src/bluesearch/database/topic_info.py Show resolved Hide resolved
src/bluesearch/database/topic_info.py Outdated Show resolved Hide resolved
src/bluesearch/database/topic_info.py Show resolved Hide resolved
Copy link
Contributor

@jankrepl jankrepl left a comment

Choose a reason for hiding this comment

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

Very reasonable! Thank you for the effort:)

src/bluesearch/entrypoint/database/topic_extract.py Outdated Show resolved Hide resolved
src/bluesearch/database/topic_info.py Outdated Show resolved Hide resolved
src/bluesearch/database/topic_info.py Outdated Show resolved Hide resolved
src/bluesearch/database/topic_info.py Outdated Show resolved Hide resolved
src/bluesearch/database/topic_info.py Show resolved Hide resolved
@FrancescoCasalegno
Copy link
Contributor

Discussed 2022-01-11

Let's wait for #535 and #540 to be merged into master before merging this PR.
Notice that the code will need to be changed to address the updates on the master.

Copy link
Contributor

@FrancescoCasalegno FrancescoCasalegno left a comment

Choose a reason for hiding this comment

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

Thank you for your work @Stannislav Stan!

It will definitely make our code simpler to maintain and understand!

src/bluesearch/entrypoint/database/topic_extract.py Outdated Show resolved Hide resolved
src/bluesearch/database/topic_info.py Outdated Show resolved Hide resolved
src/bluesearch/entrypoint/database/topic_extract.py Outdated Show resolved Hide resolved
src/bluesearch/database/topic_info.py Show resolved Hide resolved
Copy link
Contributor

@FrancescoCasalegno FrancescoCasalegno left a comment

Choose a reason for hiding this comment

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

Lgtm!

@FrancescoCasalegno FrancescoCasalegno merged commit 5ed9701 into master Jan 19, 2022
@FrancescoCasalegno FrancescoCasalegno deleted the issues/539-topic-info-data-structure branch January 19, 2022 13:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a data structure for topic info
4 participants