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

updated tqdm to print files/sec instead of it/sec #248

Merged
merged 9 commits into from
Sep 13, 2024
Merged

Conversation

lnrekerle
Copy link
Collaborator

@lnrekerle lnrekerle commented Sep 5, 2024

Individuals Processed: 100%|██████████| 337/337 [01:38<00:00, 3.43individuals/s]

This is what the bar should look like now.
This will close #241

Copy link
Member

@ielis ielis left a comment

Choose a reason for hiding this comment

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

We should probably use "individuals" as a unit. Phenopackets do not necessarily have to only come from a file. Take Phenopacket Store as an example. There is one ZIP archive that is open and the phenopacket data is read from the archive file on the fly..

@lnrekerle
Copy link
Collaborator Author

Oh! Yah, okay, should we do individual or patient? Since the start of the bar does say "Patients Created", using patient would make sense.

@ielis

@pnrobinson
Copy link
Member

Please alway replace the word "patient" with the work "individual".
People with rare disease want this because individual captures better the fact that affected people have lives outside of being a patient (which is a role we all play when we go to the doctor, but is not our individual identity).

@lnrekerle
Copy link
Collaborator Author

@pnrobinson
That makes a lot of sense! How does this loading bar look then?
Individuals Processed: 100%|██████████| 337/337 [01:38<00:00, 3.43individuals/s]

@lnrekerle lnrekerle requested a review from ielis September 6, 2024 14:59
@ielis
Copy link
Member

ielis commented Sep 9, 2024

@lnrekerle yes, the progress bar looks good!

Copy link
Member

@ielis ielis left a comment

Choose a reason for hiding this comment

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

So, the progress prompt still needs to be aligned with the prompt from the PR messages.

Copy link
Member

@ielis ielis left a comment

Choose a reason for hiding this comment

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

See the comment below.

src/gpsea/preprocessing/_config.py Outdated Show resolved Hide resolved
Copy link
Member

@ielis ielis left a comment

Choose a reason for hiding this comment

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

Yes, this is it.

@ielis ielis merged commit 1e3b011 into develop Sep 13, 2024
3 checks passed
@ielis ielis deleted the it_to_files branch September 13, 2024 07:50
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

Successfully merging this pull request may close these issues.

Patients Created: 56it [00:54, 1.03it/s]
3 participants