pepkit / peppy

Project metadata manager for PEPs in Python
https://pep.databio.org/peppy
BSD 2-Clause "Simplified" License
37 stars 13 forks source link

changes in the way of initialization Project from dict and pandas df #405

Closed khoroshevskyi closed 2 years ago

khoroshevskyi commented 2 years ago

Address #402 Also addresses #401

Big changes in functions:

nsheff commented 2 years ago

Is the purpose of this to simplify inserting into the database with pepdbagent?

I mean, what prompted the change?

khoroshevskyi commented 2 years ago

Is the purpose of this to simplify inserting into the database with pepdbagent?

Not simplifying, I would say make it more efficient (especially storage efficient) and more reasonable to store one sample table, then 3 sample tables in different configurations. I assume, now it will take 3x less space in pepdb.

nsheff commented 2 years ago

and will this also make it possible to use simplified projects, because they are unprocessed? like, I mean collapsed constant rows to the project level?

khoroshevskyi commented 2 years ago

and will this also make it possible to use simplified projects, because they are unprocessed? like, I mean collapsed constant rows to the project level?

Do you mean projects, such as geofetch is producing (e.g. where config file contains append)? - then Yes!

nsheff commented 2 years ago

Do you mean projects, such as geofetch is producing (e.g. where config file contains append)? - then Yes!

Yes, that's what I mean. Great!

Can you rename this Pull Request to something more clear? Why does it say "402" ?

rafalstepien commented 2 years ago

Do you mean projects, such as geofetch is producing (e.g. where config file contains append)? - then Yes!

Yes, that's what I mean. Great!

Can you rename this Pull Request to something more clear? Why does it say "402" ?

I think because it originated from issue number 402

nsheff commented 2 years ago

I think because it originated from issue number 402

Indeed. @Khoroshevskyi you have to use the # sign to link to the issue and indicate it as an issue number. Also, this probably belongs in the description of the PR

rafalstepien commented 2 years ago

So after changing the name of PR are we good to go?

nsheff commented 2 years ago

~I made 1 request for a change, this still is not addressed (see above)~

Nevermind, I was wrong.

khoroshevskyi commented 2 years ago

Do you mean projects, such as geofetch is producing (e.g. where config file contains append)? - then Yes!

Yes, that's what I mean. Great!

Can you rename this Pull Request to something more clear? Why does it say "402" ?

As Rafal mentioned above, the name was originated from the issue number 402. All 4 changes are related to this problem. eq function is related to functionality to_dict; from_dict is dependent from to_dict ; And from_pandas has similar function as from_dict . That's why all this changes were made here. Is there something else to change @nsheff ?

khoroshevskyi commented 2 years ago

I have chenged the names of the dict keys, could you check it please? @nsheff --> https://github.com/pepkit/peppy/pull/405/commits/802ea1a112635f1f233a5faa12e0716bf13d8157