Skip to content

Polish EIC data files - #2039

Merged
RoyStegeman merged 10 commits into
masterfrom
240408_polish_filterfiles_1
Apr 11, 2024
Merged

Polish EIC data files#2039
RoyStegeman merged 10 commits into
masterfrom
240408_polish_filterfiles_1

Conversation

@comane

@comane comane commented Apr 8, 2024

Copy link
Copy Markdown
Member

This PR aims at polishing the ATHENA data files.
Things that are done:

  • moved all xlsx files into a rawdata folder
  • added openpyxl to poetry extras. This is needed by pd.read_excel
  • created an athena_utils.py file so as to minimise repeated code.

@comane

comane commented Apr 8, 2024

Copy link
Copy Markdown
Member Author

@scarlehoff @Radonirinaunimi @RoyStegeman I think we should probably have a nnpdf_data/filter_utils folder that contains all filter utils modules for datasets. What do you think

@RoyStegeman

Copy link
Copy Markdown
Member

Thanks, I'll have a closer look later, but I agree that, if it makes sense to have multiple filter utils with e.g. some corresponding to only a single experiment, it would be good to have a filter_utils folder. That's exactly the sort of thing that's needed

@Radonirinaunimi

Copy link
Copy Markdown
Member

Thanks so much for this @comane!

@scarlehoff @Radonirinaunimi @RoyStegeman I think we should probably have a nnpdf_data/filter_utils folder that contains all filter utils modules for datasets. What do you think

Yes, something along that line should definitely be adopted, and as a matter of fact I was planning to discuss with you (later this week) about the main utils that were/are required in the datasets you implemented in order to draft something.

In this respect, as far as this PR is concerned, you could already move athena_ultis.py in a folder nnpdf_data/filter_utils.

Comment thread nnpdf_data/nnpdf_data/new_commondata/ATHENA_NC_105GEV_EP/filter.py Outdated
Comment thread nnpdf_data/nnpdf_data/athena_utils.py Outdated
Comment thread nnpdf_data/nnpdf_data/athena_utils.py Outdated
Comment thread nnpdf_data/nnpdf_data/athena_utils.py Outdated
Comment thread nnpdf_data/nnpdf_data/filter_utils/athena_utils.py Outdated
@Radonirinaunimi Radonirinaunimi added the Polarised Polarised PDF fits label Apr 9, 2024
Comment thread nnpdf_data/nnpdf_data/filter_utils/athena_utils.py Outdated
@Radonirinaunimi Radonirinaunimi changed the title Polish ATHENA data files Polish EIC data files Apr 9, 2024
@Radonirinaunimi

Copy link
Copy Markdown
Member

@comane, @RoyStegeman, given that these Athena share quite a lot of the functions with the other EIC projections, I also fixed them here. I therefore renamed the module from athena_utils $\longrightarrow$ eic_utils.

Comment thread nnpdf_data/nnpdf_data/filter_utils/README.md Outdated
@RoyStegeman

Copy link
Copy Markdown
Member

Please rebase so the tests will pass

Comment thread nnpdf_data/nnpdf_data/filter_utils/eic_utils.py Outdated

@RoyStegeman RoyStegeman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, seems fine to me

Comment thread nnpdf_data/nnpdf_data/filter_utils/eic_utils.py Outdated
@Radonirinaunimi
Radonirinaunimi force-pushed the 240408_polish_filterfiles_1 branch 2 times, most recently from 1dc9baf to ec7c090 Compare April 11, 2024 10:04

@Radonirinaunimi Radonirinaunimi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@comane I think this is done now, please merge if you are fine with this.

@comane

comane commented Apr 11, 2024

Copy link
Copy Markdown
Member Author

thanks @Radonirinaunimi and @RoyStegeman for reviewing!

@comane
comane force-pushed the 240408_polish_filterfiles_1 branch from 3b6cf1d to ee8921f Compare April 11, 2024 16:54
@RoyStegeman

Copy link
Copy Markdown
Member

You only force-pushed your local changes. I still have the commits from the review locally, would you like me to push those?

@comane

comane commented Apr 11, 2024

Copy link
Copy Markdown
Member Author

Yes please, I might have done a mess.
I had rebased on master but perhaps I didn't pull from the branch :(

@RoyStegeman
RoyStegeman force-pushed the 240408_polish_filterfiles_1 branch from ee8921f to 3b6cf1d Compare April 11, 2024 17:12
@RoyStegeman
RoyStegeman force-pushed the 240408_polish_filterfiles_1 branch from 3b6cf1d to f56203d Compare April 11, 2024 17:12
@RoyStegeman
RoyStegeman merged commit 7fb6b69 into master Apr 11, 2024
@RoyStegeman
RoyStegeman deleted the 240408_polish_filterfiles_1 branch April 11, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants