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

Could we drop the parenthetical portion of the cur_land_use permissible values? #60

Closed
Tracked by #587
turbomam opened this issue Feb 19, 2022 · 17 comments
Closed
Tracked by #587
Assignees

Comments

@turbomam
Copy link
Member

I have code that ignores the parenthetical portion when mapping to EnvO terms, but even displaying them in the DH pulldowns is awkward

  • badlands
  • cities
  • conifers (e.g. pine,spruce,fir,cypress)
  • crop trees (nuts,fruit,christmas trees,nursery trees)
  • farmstead
  • gravel
  • hardwoods (e.g. oak,hickory,elm,aspen)
  • hayland
  • horticultural plants (e.g. tulips)
  • industrial areas
  • intermixed hardwood and conifers
  • marshlands (grass,sedges,rushes)
  • meadows (grasses,alfalfa,fescue,bromegrass,timothy)
  • mines/quarries
  • mudflats
  • oil waste areas
  • pastureland (grasslands used for livestock grazing)
  • permanent snow or ice
  • rainforest (evergreen forest receiving >406 cm annual rainfall)
  • rangeland
  • roads/railroads
  • rock
  • row crops
  • saline seeps
  • salt flats
  • sand
  • shrub crops (blueberries,nursery ornamentals,filberts)
  • shrub land (e.g. mesquite,sage-brush,creosote bush,shrub oak,eucalyptus)
  • small grains
  • successional shrub land (tree saplings,hazels,sumacs,chokecherry,shrub dogwoods,blackberries)
  • swamp (permanent or semi-permanent water body dominated by woody plants)
  • tropical (e.g. mangrove,palms)
  • tundra (mosses,lichens)
  • vegetable crops
  • vine crops (grapes)
@mslarae13
Copy link

I'm good with it. Maybe put the text that's in the () in the display hint?
However, I believe we're currently using EXACTLY what MIxS says is appropriate. I'd like to hear @cmungall 's thoughts on how to convey that we are technically diverging from MIxS here if we do.

@turbomam let me know if you need anything else from me

@turbomam
Copy link
Member Author

Maybe put the text that's in the () in the display hint?

Excellent

I'd like to hear @cmungall 's thoughts on how to convey that we are technically diverging from MIxS here if we do.

@cmungall and I discussed this informally in the past and he didn't explicitly object. But yes, it would be good to get explicit sign-off. I would move any column for a slot whose permissible values were changed into the MiXS (modified) section, but that wouldn't make it explicit that the permissible values in the enumeration have been changed.

I also intend to raise this as an issue with GSC.

@turbomam
Copy link
Member Author

2023-05-18: yes, remove the parenthetical portions form the PV texts

put the parenthetical examples as annotations on the permissible values

@turbomam
Copy link
Member Author

turbomam commented May 19, 2023

I am going to do this manually now and make an issue for a better approach in the future. See

@turbomam
Copy link
Member Author

turbomam commented May 19, 2023

The PV are not asserted in schemasheets/tsv_in/enums.tsv

They are inherited via

local/with_shuttles.yaml: local/from_schemasheets.yaml \
sheets_and_friends/tsv_in/import_slots_regardless.tsv
		$(RUN) do_shuttle \
			--config_tsv  $(word 2,$^) \
			--recipient_model $(word 1,$^) \
			--yaml_output [email protected]

@ssarrafan
Copy link

@turbomam does this mean this issue is done (manually) and can be closed?

@turbomam
Copy link
Member Author

Re #60 (comment)

No, still in progress

@turbomam
Copy link
Member Author

The nmdc-schema build process does customize the cur_land_use_enum now.

That has been propagated to the submission-schema ...

... and can be seen in the 'current land use' column in the submission schema playground

I think @pkalita-lbl and I have to make releases of those two schemas and hand that work off to the Submission Portal people. In the long run, we have been talking bout having more automation with respect to that build/release/handoff process.

@turbomam
Copy link
Member Author

Note that since the examples (like 'pine') have been broken out of the permissible value names (like conifers), they aren't exposed in DataHarmoizer now. We have help pop-ups for slots, but not for individual permissible values.

@turbomam turbomam moved this from 🏗 In progress to 👀 In review/Pending Release in SubPort Squad Issues Jun 1, 2023
@ssarrafan
Copy link

Based on recent comments assuming this is active, moving to new sprint. @turbomam

@ssarrafan
Copy link

This GH issue is over 1 year old. @turbomam does this question still need to be answered? What's the goal for this issue?

@mslarae13
Copy link

@turbomam is the only thing left to go here to get this into prod on the submission portal?

@pkalita-lbl ?

@pkalita-lbl
Copy link
Collaborator

The updated schema is on dev. Is there an expectation that we modify existing submissions in the portal to match the new values? So, for example, if an in-progress submission uses "crop trees (nuts,fruit,christmas trees,nursery trees)", change it to "crop trees" for them?

@mslarae13
Copy link

@pkalita-lbl good question! Will keeping it as "crop trees (nuts,fruit,christmas trees,nursery trees)" validate? cause issues downstream / with database transforming and sharing?

@pkalita-lbl
Copy link
Collaborator

No, without writing a migration once the new version of the submission schema is brought into the portal old submissions with "crop trees (nuts,fruit,christmas trees,nursery trees)" in them already will not validate anymore. It could potentially confuse the submission-portal-to-Mongo pipeline, but we wouldn't run that on a submission that doesn't validate in the first place.

Since the migration logic is quite straightforward (replace these old values with these new values) in this case I'll go ahead and include it when I bring the latest submission schema into the portal.

@pkalita-lbl
Copy link
Collaborator

This is moving through the system as part of microbiomedata/nmdc-server#992

@ssarrafan
Copy link

Closing this based on @pkalita-lbl last comment.
Backlog cleanup 12-2023

@github-project-automation github-project-automation bot moved this from 👀 In review/Pending Release to ✅ Done in SubPort Squad Issues Dec 21, 2023
@ssarrafan ssarrafan removed the backlog label Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ SubPort 1 - Done
Development

No branches or pull requests

5 participants