Skip to content

Addressing SVG and YAML issues from #90#102

Merged
DunklesArchipel merged 102 commits into
echemdb:mainfrom
DunklesArchipel:metadatacleaning
Jan 24, 2022
Merged

Addressing SVG and YAML issues from #90#102
DunklesArchipel merged 102 commits into
echemdb:mainfrom
DunklesArchipel:metadatacleaning

Conversation

@DunklesArchipel

@DunklesArchipel DunklesArchipel commented Jan 8, 2022

Copy link
Copy Markdown
Member

This PR addresses the most critical issues raised in #90.

The latest not yet merged literature up to #91 are included.

You can quickly browse through a few of the 250+ files. Replacements are always identical. If you think we need any other changes let me know.

  • Personally, I suggest keeping https://doi.org/ and also other instances with a http preamble, unless there is a good reason that we should not do that. With the http preamble these keys in the files become clickable in most editors, which I find very convenient.

  • I would also change all filenames such that the figure identifier is always *_f1a_*. Currently, in some cases, it is only 1 or Fig1, etc..


Note to reviewers:

  • Some of the general code has been adapted to match the new metadata scheme. Please check if there are some erroneous additions in the py and md files.
  • Feel free to briefly check a few of the yaml files. I am pretty sure not all issues are resolved. We will find more mistakes, once we have a proper schema against which we can check the yaml files.

Dependencies:

@DunklesArchipel DunklesArchipel linked an issue Jan 8, 2022 that may be closed by this pull request
44 tasks
@DunklesArchipel DunklesArchipel marked this pull request as draft January 8, 2022 17:21
@DunklesArchipel DunklesArchipel marked this pull request as ready for review January 9, 2022 15:38
@DunklesArchipel DunklesArchipel marked this pull request as draft January 12, 2022 22:31
Comment thread echemdb/data/cv/entry.py Outdated
@DunklesArchipel DunklesArchipel marked this pull request as ready for review January 22, 2022 22:42
@saraedum

saraedum commented Jan 23, 2022

Copy link
Copy Markdown
Member

@DunklesArchipel there is a merge conflict now. I had a quick look but I was not sure which of the two versions we want now.

@DunklesArchipel

Copy link
Copy Markdown
Member Author

@DunklesArchipel there is a merge conflict now. I had a quick look but I was not sure which of the two versions we want now.

it's solved

source:
supplier: Merck, Suprapur, 99.99%
sum formula: LiF
supplier: Merck, Suprapur

@saraedum saraedum Jan 24, 2022

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.

Isn't the supplier just Merck? and Suprapur the brand name?

Anyway, I am also not sure what exactly is the meaning "supplied purity" and why this is not only called purity. And then why is the subkey "grade" needed at all. Couldn't it just be:

purity:
  value: 99.99
  unit: pct

But then it should probably not live under source.

But it also doesn't really matter. Just something I stumbled upon.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I will address this aspect in #115.
@linuxrider and I had already a number of discussions about this aspect.
We did not yet consider the brand name

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Merck for example calls "Suprapur©" a grade.
Maybe our purity can hold a grade string.

Comment thread literature/endo_1999_in-situ_19/endo_1999_in-situ_19_p1_f1b_bold.yaml Outdated
@saraedum

Copy link
Copy Markdown
Member

@DunklesArchipel I looked randomly at some of the changes. But I didn't look at every single line. Maybe @linuxrider wants to have another quick look.

@linuxrider linuxrider left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question about the format of the dates has to be addressed before merging.
It is a string in some cases and in other it seems to be of format date (yyyy-mm-dd).
The rest of the changes look good.

@DunklesArchipel DunklesArchipel merged commit 43ce8a4 into echemdb:main Jan 24, 2022
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.

Fix data bugs in literature and SVG Files Add page number to briega-martos_2021_cation_XXX

3 participants